Skip to content

🤖 Compare If-None-Match as a list, per rfc9110 sec 13.1.2 - #1537

Closed
rawsun007 wants to merge 1 commit into
bottlepy:masterfrom
rawsun007:fix/if-none-match-list
Closed

rawsun007 wants to merge 1 commit into
bottlepy:masterfrom
rawsun007:fix/if-none-match-list

Conversation

@rawsun007

Copy link
Copy Markdown

static_file compares the whole If-None-Match header against the entity-tag with ==, so only a client echoing the tag byte for byte gets a 304. Measured against a file it serves:

If-None-Match before rfc9110
<etag> 304 304
* 200 304
"<etag>" 200 304
"other", <etag> 200 304
W/<etag> 200 304
"nope" 200 200

The header is a comma-separated list of entity-tags or a lone * (sec 13.1.2), and sec 8.8.3.2 requires the weak comparison for it, so the surrounding quotes and any W/ prefix are not part of the value. Every row above except the first and last silently defeated caching — the client asked "only send it if it changed", and got the whole file back.

parse_etag_list and etag_matches are module-level so they can be reused by an If-Match implementation later, and are named in the same style as the existing parse_date / parse_range_header.

Two decisions worth your eye:

  • * now answers 304 whenever the file exists, including when the caller passed etag=False, because it asks whether any representation exists rather than comparing tags. That is what the spec says, but it does mean etag=False no longer suppresses every conditional. Say the word and I'll gate it on an ETag being present instead.
  • I did not change the emitted tag, which is unquoted and so not a valid entity-tag per sec 8.8.3 (an opaque-tag is a quoted-string). Comparing after normalisation means both spellings match, so this fixes the behaviour without changing a header that callers may already depend on. Worth a separate PR if you want it.

Verification: python3 -m pytest test — 372 pass. Reverting only bottle.py fails exactly the two new tests and nothing else. The new cases cover *, quoted, W/ in both spellings, a list on either side of the tag, and the non-matching cases including an empty header.

Existing behaviour I checked stays put: test_etag_overrides_ims still passes, so If-None-Match still takes precedence over If-Modified-Since as of b73bd1d.

Per AGENTS.md: this contribution is released into the public domain; AI usage is disclosed in the commit's ai-assisted-by: trailer (Claude Code, Claude Opus 5). @rawsun007 reviewed the diff, ran the suite, and authorised the sign-off. This is our only open AI-assisted PR here.

static_file compared the whole header against the entity-tag with ==, so only
a client echoing the tag byte for byte got a 304. Measured against a file
served by static_file:

    If-None-Match: <etag>          304   (worked)
    If-None-Match: *               200   should be 304
    If-None-Match: "<etag>"        200   should be 304
    If-None-Match: "other", <etag> 200   should be 304
    If-None-Match: W/<etag>        200   should be 304

The header is a comma-separated list of entity-tags or a lone `*`, and
sec 8.8.3.2 requires the weak comparison for it, so the quotes and any `W/`
prefix are not part of the value being compared. Every line above except the
first silently defeated caching.

`*` now answers 304 whenever the file exists, including when the caller passed
etag=False, because it asks whether any representation exists rather than
comparing tags.

Not changed here: static_file emits the tag unquoted, which rfc9110 sec 8.8.3
does not allow (an opaque-tag is a quoted-string). Comparing after
normalisation means both spellings work, so this commit fixes the matching
without changing a header that callers may already depend on. Happy to follow
up on the emitted form separately.

This contribution is released into the public domain.

ai-assisted-by: Claude Code (Claude Opus 5)
Signed-off-by: Roshan Ramani <roshanramani.dev@gmail.com>
@defnull

defnull commented Sep 6, 2026

Copy link
Copy Markdown
Member

Hmm, looks like static_file indeed generated invalid etag headers, and does not support multi-valued INM headers, but:

  • Your parse_etag_list implementation is broken and does not conform to rfc9110. The flaw is so obvious that I doubt the diff was properly reviewed by a human.
  • The actual issue (unquoted etags) is not fixed. The backwards compatibility claim does not hold in this context.
  • There is a WAY simpler solution that is also correct in the narrowed context of static_file.

@defnull defnull closed this Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants