HTTP caching: Cache-Control, ETag, and conditional requests

docker/include/proxy_cache.conf

@@ -30,11 +30,25 @@ proxy_cache_revalidate on;
 # Ranged responses must still stay out of the cache: proxy_cache_key has no
 # Range in it, so caching a 206 would serve those bytes for every other range
 # of the same object. Unranged GETs (zarr.json, bucket listings) still cache.
-proxy_cache_bypass $http_range;
+# $cache_bypass (Cache-Control: no-cache) is repeated from proxy_cache_maps.conf
+# rather than left at http level: a location-level proxy_cache_bypass replaces
+# the http-level one instead of adding to it, so naming only $http_range here
+# would silently drop the no-cache policy in every location that includes this
+# file.
+proxy_cache_bypass $http_range $cache_bypass;
 proxy_no_cache     $http_range;
 proxy_set_header   Range    $http_range;
 proxy_set_header   If-Range $http_if_range;
 
+# Same story for the other conditional headers: proxy_cache makes nginx replace
+# them with its own revalidation values, so on a cache-bypassed ranged request
+# the client's validators never reach x2s3 and it cannot answer the 304 that
+# RFC 9110 13.2.2 requires. These variables restore them for ranged requests
+# only and leave nginx's own revalidation alone otherwise -- see
+# proxy_cache_maps.conf, which defines them and explains why.
+proxy_set_header   If-None-Match     $proxy_if_none_match;
+proxy_set_header   If-Modified-Since $proxy_if_modified_since;
+
 # Set back a nice HTTP Header to indicate what the cache status was
 add_header X-Proxy-Cache $upstream_cache_status;
 

docker/include/proxy_cache_maps.conf

@@ -0,0 +1,39 @@
+# Cache-related maps, split out because map is http-context only. Included from
+# nginx.conf; consumed by proxy_cache.conf.
+
+# A client asking for a fresh copy with Cache-Control: no-cache should not be
+# handed nginx's stored one. Only proxy_cache_bypass is set from this, not
+# proxy_no_cache: no-cache means "revalidate before serving", so the response
+# should still refresh the cache entry.
+map $http_cache_control $cache_bypass {
+    no-cache   1;
+}
+
+# The remaining maps restore conditional-request headers on ranged requests.
+#
+# Configuring proxy_cache makes nginx replace the client's If-None-Match and
+# If-Modified-Since with its own revalidation values ($upstream_cache_etag and
+# $upstream_cache_last_modified). Like the Range stripping described in
+# proxy_cache.conf, that is decided at config time, so it applies even to the
+# ranged requests proxy_cache_bypass sends straight through -- there the
+# client's validators are dropped and x2s3 never sees them, so it cannot answer
+# the 304 that RFC 9110 13.2.2 requires when Range and If-None-Match arrive
+# together.
+#
+# Restoring them for ranged requests only is deliberate. On an unranged request
+# nginx owns these headers: with proxy_cache_revalidate on, a stale entry is
+# revalidated using the ETag nginx itself cached. Forwarding the client's
+# validator there would let it answer nginx's question, so a 304 meant for the
+# client would mark a stale entry fresh and keep serving its old bytes for
+# another proxy_cache_valid period.
+#
+# The '' branches reproduce nginx's built-in defaults exactly.
+map $http_range $proxy_if_none_match {
+    ''      $upstream_cache_etag;
+    default $http_if_none_match;
+}
+
+map $http_range $proxy_if_modified_since {
+    ''      $upstream_cache_last_modified;
+    default $http_if_modified_since;
+}

docker/include/proxy_pass.conf

@@ -13,9 +13,11 @@ proxy_ignore_headers X-Accel-Expires;
 proxy_ignore_headers Cache-Control;
 proxy_ignore_headers Set-Cookie;
 
+# Cache-Control is intentionally NOT hidden here: x2s3 emits it so browsers
+# and shared caches can cache responses. proxy_ignore_headers Cache-Control
+# above still keeps nginx's own proxy_cache_valid in force regardless.
 proxy_hide_header Expires;
 proxy_hide_header X-Accel-Expires;
-proxy_hide_header Cache-Control;
 proxy_hide_header Pragma;
 
 # Replace CORS headers

docker/nginx.conf

@@ -11,10 +11,11 @@ events {
 http {
     proxy_cache_path /var/cache/nginx keys_zone=mycache:512m max_size=100g levels=1:2 inactive=24h
                      loader_sleep=10ms manager_files=4000 manager_threshold=200m manager_sleep=100ms;
-    map $http_cache_control $cache_bypass {
-        no-cache   1;
-    }
-    proxy_cache_bypass $cache_bypass;
+    # map is http-context only, so the maps consumed by proxy_cache.conf have
+    # to be included out here. The matching proxy_cache_bypass lives in
+    # proxy_cache.conf: a location-level one replaces an http-level one rather
+    # than adding to it, so both policies have to be named together there.
+    include /etc/nginx/conf/proxy_cache_maps.conf;
 
     # Increase in-memory buffers (this assumes we have a lot of RAM)
     proxy_buffer_size 64k;

docs/Config.md

@@ -19,7 +19,6 @@ client_options:
     max_pool_connections: 50
   file:
     buffer_size: 65536  # 64 KB chunks for streaming
-    calculate_etags: false
 ```
 
 ## Targets
@@ -41,7 +40,6 @@ Each target may have the following properties:
     * *file*: Local filesystem targets. Options:
         * `path`: Path to the root
         * `buffer_size`: Size of chunks (in bytes) when streaming file content (default: 8192)
-        * `calculate_etags`: If true, then the etags will be calculated by hashing the content of each file. This is much more expensive and may not be needed for all use cases.
 
 ### Botocore Config Options
 

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "x2s3"
-version = "1.4.3"
+version = "1.4.4"
 description = "RESTful web service which makes any storage system X available as an S3-compatible REST API"
 readme = "README.md"
 license = { file = "LICENSE" }

tests/test_awss3.py

@@ -160,6 +160,7 @@ def test_head_object(app):
     with TestClient(app) as client:
         response = client.head("/janelia-data-examples/jrc_mus_lung_covid.n5/attributes.json")
         assert response.status_code == 200
+        assert response.headers['cache-control'] == "public, max-age=3600"
         response = client.head("/janelia-data-examples/jrc_mus_lung_covid.n5/")
         assert response.status_code == 404
 
@@ -178,6 +179,7 @@ def test_get_object(app):
     with TestClient(app) as client:
         response = client.get("/janelia-data-examples/jrc_mus_lung_covid.n5/attributes.json")
         assert response.status_code == 200
+        assert response.headers['cache-control'] == "public, max-age=3600"
         json_obj = response.json()
         assert 'n5' in json_obj
 

tests/test_boto.py

@@ -1,12 +1,17 @@
+import asyncio
 import time
 import multiprocessing
+from datetime import datetime, timezone
+from email.utils import parsedate_to_datetime
 
 import boto3
 import pytest
 from pydantic import HttpUrl
 
 from x2s3.app import create_app
+from x2s3.client_aioboto import AiobotoProxyClient
 from x2s3.settings import Target, Settings
+from x2s3.utils import CACHE_CONTROL_PUBLIC
 
 # Set the start method to spawn to avoid pickling issues
 multiprocessing.set_start_method('spawn', force=True)
@@ -165,3 +170,170 @@ def test_get_object_precedence(app, s3_client):
     assert response['ResponseMetadata']['HTTPStatusCode'] == 200
     json_obj = response['Body'].read().decode('utf-8')
     assert 'n5' in json_obj
+
+
+# The tests below drive AiobotoProxyClient against a stub and need no network.
+
+
+class _StubS3:
+    """Minimal stand-in for the aiobotocore client, recording its calls."""
+
+    def __init__(self, get_headers=None, head_response=None, get_error=None):
+        self.get_headers = get_headers or {}
+        self.head_response = head_response or {}
+        self.get_error = get_error
+        self.calls = []
+
+    async def get_object(self, **kwargs):
+        self.calls.append(kwargs)
+        if self.get_error is not None:
+            raise self.get_error
+        return {"ResponseMetadata": {"HTTPHeaders": self.get_headers},
+                "Body": None}
+
+    async def head_object(self, **kwargs):
+        self.calls.append(kwargs)
+        return self.head_response
+
+
+def call_with_stub(stub, method, *args, **options):
+    """Await one AiobotoProxyClient method against a stub S3 client.
+
+    The client is built inside the coroutine on purpose: asyncio.Lock() binds
+    to the running loop on Python 3.9 and asyncio.run() leaves no current loop
+    behind, so building it outside would break whichever test ran second.
+    """
+    async def run():
+        client = AiobotoProxyClient({'target_name': 'test'},
+                                    bucket='test-bucket', **options)
+        client.client = stub
+        return await getattr(client, method)(*args)
+
+    return asyncio.run(run())
+
+
+def test_head_object_last_modified_is_locale_independent():
+    # botocore hands head_object a real datetime, and formatting it with
+    # strftime('%a, %d %b %Y ...') expands %a/%b using the process locale --
+    # under e.g. LC_TIME=de_DE that yields '.., 18 Dez ...', which no cache or
+    # client can parse, silently killing 304 revalidation for S3 targets. No
+    # non-English locale is installed in CI, so simulate one with a datetime
+    # whose strftime is locale-poisoned.
+    class GermanLocaleDatetime(datetime):
+        def strftime(self, fmt):
+            return super().strftime(fmt).replace('Dec', 'Dez')
+
+    last_modified = GermanLocaleDatetime(2026, 12, 18, 12, 0, 0,
+                                         tzinfo=timezone.utc)
+    stub = _StubS3(head_response={"ContentLength": 1234,
+                                  "LastModified": last_modified})
+
+    response = call_with_stub(stub, 'head_object', 'some/key.json')
+    assert parsedate_to_datetime(response.headers['last-modified']) == last_modified
+
+
+def test_get_object_keeps_upstream_cache_control():
+    # An object the origin marked uncacheable must not be re-advertised as
+    # publicly cacheable for an hour: the browser and the shared nginx cache in
+    # front of x2s3 would both retain content the origin said not to store.
+    stub = _StubS3(get_headers={"content-length": "10",
+                                "cache-control": "no-store"})
+    handle = call_with_stub(stub, 'open_object', 'some/key.json')
+    assert handle.headers["Cache-Control"] == "no-store"
+
+
+def test_get_object_defaults_cache_control_when_upstream_has_none():
+    stub = _StubS3(get_headers={"content-length": "10"})
+    handle = call_with_stub(stub, 'open_object', 'some/key.json')
+    assert handle.headers["Cache-Control"] == CACHE_CONTROL_PUBLIC
+
+
+def test_head_object_keeps_upstream_cache_control():
+    stub = _StubS3(head_response={
+        "ContentLength": 10,
+        "LastModified": datetime(2026, 8, 18, 12, 0, 0, tzinfo=timezone.utc),
+        "CacheControl": "private, max-age=60",
+    })
+    response = call_with_stub(stub, 'head_object', 'some/key.json')
+    assert response.headers["cache-control"] == "private, max-age=60"
+
+
+NOT_MODIFIED_HEADERS = {"etag": '"upstream-etag"',
+                        "last-modified": "Fri, 26 Jul 2024 13:39:10 GMT"}
+
+
+def _not_modified_error():
+    """The ClientError real S3 raises for a conditional GET that matches."""
+    from botocore.exceptions import ClientError
+    return ClientError({"Error": {"Code": "304", "Message": "Not Modified"},
+                        "ResponseMetadata": {"HTTPStatusCode": 304,
+                                             "HTTPHeaders": NOT_MODIFIED_HEADERS}},
+                       "GetObject")
+
+
+def test_open_object_forwards_conditional_headers_to_s3():
+    # Without this the proxy fetches the whole object upstream and throws the
+    # body away to answer 304, so a client revalidating thousands of cached
+    # chunks costs nearly as much as never having cached them.
+    stub = _StubS3(get_headers={"content-length": "10"})
+    call_with_stub(stub, 'open_object', 'some/key.json', None,
+                   '"client-etag"', 'Fri, 26 Jul 2024 13:39:10 GMT')
+    assert stub.calls[0]["IfNoneMatch"] == '"client-etag"'
+    assert stub.calls[0]["IfModifiedSince"] == parsedate_to_datetime(
+        "Fri, 26 Jul 2024 13:39:10 GMT")
+
+
+def test_open_object_returns_304_when_s3_says_not_modified():
+    stub = _StubS3(get_error=_not_modified_error())
+    response = call_with_stub(stub, 'open_object', 'some/key.json', None,
+                              '"upstream-etag"', None)
+    assert response.status_code == 304
+    assert response.headers["last-modified"] == NOT_MODIFIED_HEADERS["last-modified"]
+    assert response.headers["cache-control"] == CACHE_CONTROL_PUBLIC
+
+
+def test_304_from_s3_carries_etag_only_when_proxied():
+    # proxy_etag=False exists to keep upstream ETags off the wire for backends
+    # whose ETags break the AWS SDK integrity check; a 304 must not leak one.
+    stub = _StubS3(get_error=_not_modified_error())
+    hidden = call_with_stub(stub, 'open_object', 'some/key.json', None,
+                            '"upstream-etag"', None)
+    assert "etag" not in hidden.headers
+
+    stub = _StubS3(get_error=_not_modified_error())
+    shown = call_with_stub(stub, 'open_object', 'some/key.json', None,
+                           '"upstream-etag"', None, proxy_etag=True)
+    assert shown.headers["etag"] == NOT_MODIFIED_HEADERS["etag"]
+
+
+def test_unparseable_if_modified_since_is_not_forwarded():
+    # botocore wants a datetime; handing it a garbage string would raise, so a
+    # slightly-off client header must not become a 500.
+    stub = _StubS3(get_headers={"content-length": "10"})
+    handle = call_with_stub(stub, 'open_object', 'some/key.json', None,
+                            None, 'not a date')
+    assert "IfModifiedSince" not in stub.calls[0]
+    assert handle.status_code == 200
+
+
+def test_conditional_get_returns_304_against_real_s3(app, s3_client):
+    # End-to-end through the running proxy: the ETag the client got back must
+    # be answerable with a 304 rather than a second full body.
+    bucket = 'janelia-data-examples-with-etag'
+    key = 'jrc_mus_lung_covid.n5/attributes.json'
+    etag = s3_client.get_object(Bucket=bucket, Key=key)['ETag']
+
+    with pytest.raises(s3_client.exceptions.ClientError) as exc_info:
+        s3_client.get_object(Bucket=bucket, Key=key, IfNoneMatch=etag)
+    assert exc_info.value.response['ResponseMetadata']['HTTPStatusCode'] == 304
+
+
+def test_head_and_get_agree_on_caching_headers_over_s3(app, s3_client):
+    # Same invariant as the file client, across the other pair of code paths.
+    bucket = 'janelia-data-examples-with-etag'
+    key = 'jrc_mus_lung_covid.n5/attributes.json'
+    head = s3_client.head_object(Bucket=bucket, Key=key)
+    get = s3_client.get_object(Bucket=bucket, Key=key)
+    assert head['ETag'] == get['ETag']
+    assert head['LastModified'] == get['LastModified']
+    assert head.get('CacheControl') == get.get('CacheControl')