Skip to content

Evaluate every RFC 9110 precondition through http's Precondition - #70

Closed
carpentry-agent[bot] wants to merge 5 commits into
mainfrom
claude/web-preconditions
Closed

carpentry-agent[bot] wants to merge 5 commits into
mainfrom
claude/web-preconditions

Conversation

@carpentry-agent

@carpentry-agent carpentry-agent Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

web now hands conditional requests to http 0.7.0's Precondition (already pinned since 0.12.0) instead of matching them itself. That adds the half of RFC 9110 §13 web never evaluated (If-Match, If-Unmodified-Since, If-Range) and deletes web's duplicate ETag matching. web's stricter date parser stays, as a gate in front of Precondition (see Kept below).

⚠️ Behaviour change for POST/PUT/PATCH/DELETE: please decide

web evaluates preconditions after the handler has run, by rewriting its response. For a GET or HEAD that is fine, because the handler changed nothing. For a state-changing method the write has already happened, and the response's ETag describes the new representation. Measured on main (751a14a) with a PUT handler that counts its writes:

request handler wrote client got
PUT + If-None-Match: * (conditional create) once 412
PUT + stale If-Match: "v0" once 201

The first row means every correct conditional create gets a 412, so the client is told its create failed after it succeeded. The second row means there is no lost-update protection. A post-hoc If-Match would not fix that. It would compare the client's old tag with the handler's new one and send a 412 to every correct conditional update, after the write.

This PR stops rewriting the response of any method other than GET/HEAD. It goes out as the handler built it. The module doc and the README (new "Conditional requests" section) now say that a handler of a state-changing method calls Request.preconditions against the resource's current validators before acting, with a short example. The test suite runs the same pattern (update-checked). There is no post-hoc If-Match/If-Unmodified-Since for those methods. #62 flagged this caveat when it introduced the 412 and named adopting Precondition as the fix.

If you would rather keep today's post-hoc If-None-Match → 412 for those methods, it needs its own special case in web-conditional. I left it out because the table's first row is what it does to every correct use.

What changed

  • Preconditions (GET/HEAD, 2xx only). web-conditional reads the selected representation's validators from the response: the first ETag via ETag.parse and the first Last-Modified via HttpDate.parse, each Nothing when absent or unreadable. Precondition.evaluate then evaluates If-Match, If-Unmodified-Since, If-None-Match and If-Modified-Since in §13.2.2 order, over the request's headers minus any date precondition web's own parser rejects. A 304 still goes through web-not-modified and a 412 through web-precondition-failed, so both keep exactly the headers they kept before. The "non-2xx ignores preconditions" gate (§13.2.1) is unchanged.
  • If-Range. Request.if-range-matches? now gates web-mark-range, so a failed If-Range serves the whole file with a 200. Before, If-Range was ignored (grep -ci 'if-match\|if-unmodified\|if-range' web.carp was 0). A client resuming a download of a static file that had changed got a 206 of the new file, which it spliced onto the old file's bytes. That breaks the MUST in §13.1.5, and static files carry both validators.
  • Deleted, now dead: web-etag-opaque, web-parse-etag-list, web-etag-list-match?, web-etag-match?, web-if-modified-since? and web-conditional-not-modified?. I checked callers first, and nothing in examples/ or bench/ uses any of them. web-bodyless, web-not-modified, web-precondition-failed and web-get-or-head? stay.
  • Kept: Read every date format an If-Modified-Since is allowed to carry #65's date parser, as a gate. HttpDate.parse reads 31 Feb as 3 March and accepts text after the date (divergences 2 and 3 below). That is exactly what web 0.11.0's changelog lists as fixed. web-strict-date-headers drops an If-Modified-Since or If-Unmodified-Since that web-parse-http-date rejects before the headers reach Precondition.evaluate, so the header is ignored, as §13.1.3 and §13.1.4 require. Once time's strptime checks the day against the month and requires the whole value to be consumed, the gate and the parser can go.

Differential: old matcher vs Precondition

Run before deleting anything. A probe loaded main's web.carp and computed both decisions over the same response: the old one (web-conditional-not-modified?) and the new one (Request.preconditions on the validators above). The inputs were every combination of:

  • 5 response ETags: "abc", W/"abc", unquoted abc, "a,b", none
  • 4 Last-Modifieds: IMF-fixdate, asctime, garbage, none
  • 20 If-None-Match values: none, *, strong, weak, lists matching on the second member, a quoted comma, "a", "", unterminated, unquoted, empty, "z", junk, "abc", *, "abc", "abc" "z", lowercase w/, a space inside the tag, W/ "abc"
  • 21 If-Modified-Since values: none; IMF-fixdate, rfc850 and asctime dates before, after and equal; a space-padded asctime day; UTC; 31 Feb; a wrong real day name; bogus day names; both dashed cookie-date forms; :60; trailing junk; a lowercase month; a one-digit IMF day; garbage
  • both GET and HEAD

That is 16,800 combinations; 1,128 diverge. They fall into five classes, identically for GET and HEAD:

# class example old new rows
1 If-None-Match with a malformed member "z", junk, "abc" · *, "abc" · "abc" "z", against ETag: "abc" 304 200 1008
2 If-Modified-Since: a day the month does not have Fri, 31 Feb 2100 00:00:00 GMT 200 304 20
3 If-Modified-Since: text after the date Fri, 31 Dec 2100 23:59:59 GMT junk 200 304 20
4 If-Modified-Since: dashed cookie-date forms Fri, 31-Dec-2100 … · Fri, 31-Dec-68 … 200 304 40
5 If-Modified-Since: a day name that is not one Xyz, 31 Dec 2100 … · Funday, 31-Dec-68 … 304 200 40
  1. The known one. web skipped the malformed members and matched the rest. ETagList.parse rejects the whole field, so If-None-Match is ignored, but because it is present it still suppresses If-Modified-Since. The new result is always a full 200, never a wrong 304, which is the safe direction.
  2. and 3. These are HttpDate.parse leniencies, not choices. Datetime.strptime does not check the day against the month (31 Feb 2100 is read as 3 Mar) and does not require the whole value to be consumed. web's parser rejected both, and Read every date format an If-Modified-Since is allowed to carry #65 pinned them. §13.1.3 says to ignore an IMS that is not a valid HTTP-date. This only produces a wrong 304 when the misread date is not older than Last-Modified. The fix belongs in time's strptime. Until then the gate above keeps web's behaviour, so on this branch neither divergence survives.
  3. Deliberate in http. The same parser reads cookie Expires, and RFC 6265 allows those forms.
  4. http is stricter here, which matches §5.6.7's grammar. web never looked at the day name.

With the date gate (second commit), only classes 1 and 5 remain. web's own parser rejects every value in classes 2, 3 and 4, so those headers are dropped before Precondition.evaluate sees them and are ignored, as they were before.

These did not diverge: UTC for GMT, a lowercase month, a one-digit IMF day, W/ "abc", lowercase w/, unquoted and unterminated tags, empty values, :60 (both read the next second), a wrong but real day name (both accept), and the space-padded asctime day (both accept).

Tests

test/web.carp: main 403/0 → branch 431/0. Each changed expectation:

  • a matching If-None-Match on PUT/DELETE returns 412 → 200, the handler's own response, per the section above. Both tests are renamed.
  • The 412-shape tests (no body, no Content-Type, keeps ETag, keeps the CORS after-hook's header, a handler header and a cookie, frames Content-Length: 0) got their 412 from a PUT with a matching If-None-Match. They now get it from a GET with a failing If-Match, with the same assertions.
  • an If-Modified-Since whose day the month does not have is ignored stays. Next to it are three new tests: a space-padded asctime IMS → 304 (the Read every date format an If-Modified-Since is allowed to carry #65 format end to end, which no test covered with a date after Last-Modified), an IMS with text after the date, and an If-Unmodified-Since naming 31 Feb. The last two are both ignored.
  • The unit tests of the deleted helpers are gone. The two lower-case response-header cases are now end-to-end tests (etag, last-modified), and so is the unterminated tag. The list-splitting cases were already covered end to end. Read every date format an If-Modified-Since is allowed to carry #65's parser tests stay, with the parser.

New tests:

  • If-Match: strong match, mismatch (412), a list matching on its second member, *, a weak request tag and a weak ETag never matching (412), HEAD, a failed If-Match winning over a matching If-None-Match, and a 404 left alone.
  • If-Unmodified-Since: before (412), after, and equal to Last-Modified, an invalid date ignored, and suppressed by If-Match.
  • If-Range on the static fixture: the current ETag (206), a changed ETag (200, full Content-Length), a weak ETag (200), an equal date (206), a different date (200), the GET range marker armed and disarmed, and If-Range without Range ignored.
  • Unsafe methods: a conditional create is answered with the handler's 201 and wrote once; If-None-Match on PUT/DELETE and a stale If-Match on PUT are left alone; a handler that evaluates Request.preconditions itself sends its own 412 without writing, and writes on a current If-Match.

Mutation check. I ran three mutants of web-conditional against the full suite, and every one is caught:

  • Evaluating preconditions for every method fails 5 tests: the conditional create, If-None-Match on PUT/DELETE, the stale If-Match, and a current If-Match lets the handler write. The last one is the naive post-hoc If-Match sending a 412 to a correct update.
  • Dropping the If-Range gate fails 5.
  • Dropping the 2xx gate fails 2.

test/websocket.carp: 155/0 on both main and the branch.

carp-fmt -c and angler: the local binaries flag both files on main too. Among the lines this PR touches, carp-fmt had two re-wraps, both applied. angler reports the same findings on main and on the branch, nothing new. CHANGELOG has a new ## Unreleased entry.


Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

carpentry-heartbeat[bot] added 2 commits October 2, 2026 19:07
A 2xx to a GET or HEAD now has If-Match, If-Unmodified-Since,
If-None-Match and If-Modified-Since evaluated by Request.preconditions
against the response's ETag and Last-Modified, and Request.if-range-matches?
gates the Range marker, so a failed If-Range serves the whole file instead
of a 206 a resuming client would splice onto an older version.

The response of any other method is no longer rewritten. Its handler has
already acted by the time web sees it, so a conditional create (PUT with
If-None-Match: *) wrote and was then answered 412. Such handlers evaluate
Request.preconditions themselves before acting, as the module doc and
README now describe.

web's own entity-tag matching and HTTP-date parser are deleted; nothing
else called them.
HttpDate.parse reads `31 Feb` as 3 March and accepts text after the
date, which is the defect web 0.11.0 fixed for If-Modified-Since. An
If-Modified-Since or If-Unmodified-Since that web-parse-http-date
rejects is now dropped before Precondition.evaluate sees it, so it is
ignored as RFC 9110 13.1.3 and 13.1.4 require. The parser, its unit
tests and the 31 Feb test are restored, with two more for text after
the date and for If-Unmodified-Since.
@carpentry-agent

Copy link
Copy Markdown
Contributor Author

Second commit (f1fd9de), pushed after reviewing the first one: web's #65 date parser is back, as a gate in front of Precondition.evaluate.

The first commit deleted it and handed If-Modified-Since to HttpDate.parse, which is Datetime.strptime over five formats. strptime neither checks the day against the month nor requires the whole value to be consumed. That reopened exactly what web 0.11.0's changelog lists as fixed: "31 Feb 1994 used to parse as 3 March". The differential's classes 2 and 3 were that regression, not just an http leniency to file somewhere.

web-strict-date-headers now drops an If-Modified-Since or If-Unmodified-Since that web-parse-http-date rejects before the headers reach Precondition.evaluate, so the header is ignored, as §13.1.3/§13.1.4 require. With it, only differential classes 1 and 5 remain. The ETag-matching helpers stay deleted. The parser and the gate can go once time's strptime validates the day and consumes the whole value.

  • test/web.carp: 431/0 locally (CARP_DIR is a local core plus carp-lang #1587, private output dir). That is 412 from the first commit, plus Read every date format an If-Modified-Since is allowed to carry #65's 16 parser unit tests and the 31 Feb end-to-end test restored, plus two new ones: an IMS with text after the date, and an If-Unmodified-Since naming 31 Feb.
  • Teeth: with the gate removed and everything else unchanged, the suite gives 428/3. The three failures are exactly those date tests: 304, 304 and 412 where 200 is expected.
  • CHANGELOG: the second Changed bullet no longer describes HttpDate.parse's leniencies, since they no longer reach web. It now names only the two behaviour changes left: a malformed If-None-Match list is ignored as a whole, and a date with a bogus day name is ignored.
  • The PR body is updated to match.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

  • On f1fd9de, with a private output directory and CARP_DIR set to the local core plus carp-lang #1587:
    • carp -x test/web.carp: 431 passed, 0 failed.
    • carp -x test/websocket.carp: 155 passed, 0 failed.
  • CI is green on both legs, including the smoke test and gendocs.
  • I checked the second commit's teeth claim myself. With web-strict-date-headers removed from web-conditional and nothing else changed, the suite gives 428 / 3. The three failures are exactly the 31 Feb IMS test, the trailing-text IMS test and the 31 Feb IUS test.

Findings

The behaviour matches the description. I built a probe of 30 request shapes against web-build-response and ran it on main (751a14a) and on this branch:

  • Before-values. On main, HEAD with Range: bytes=0-4 and If-Range: "changed" on the static fixture gets a 206, and the same GET arms X-Sendfile-Range. The branch gives 200 and does not arm it. So the CHANGELOG's "it used to get a 206" is accurate. A PUT with If-None-Match: * gets 412 on main and the handler's 200 here, as the body's table says.
  • Header-name case. Lower-case if-none-match, if-match, if-modified-since, if-unmodified-since, range and if-range each behave like the canonical spelling. So do IF-MODIFIED-SINCE and if-Unmodified-Since carrying 31 Feb, so the gate's lower-cased compare works for both date headers.
  • Two field lines. Two If-None-Match lines, or two If-Match lines, matching on the second line give 304 and 200. Precondition.list-header joins the lines, as main's loop over all values did.
  • * without a usable ETag. With no ETag or a malformed one, If-Match: * gives 200 and If-None-Match: * gives 304, the same as main.
  • Range with a matching If-None-Match. This gives 304 with neither Content-Range nor X-Sendfile-Range, on both sides.
  • Other methods. POST and OPTIONS with a stale If-Match are left alone.

1. Non-blocking, but measurable: the GET path is about 31% slower, and two thirds of that is avoidable

The benchmark was 20,000 GETs through web-build-response. Each request had nine typical headers, including a non-matching If-None-Match, and went to a route whose 200 carries ETag and Last-Modified. Each variant is one binary built from its own tree. I ran each three times, interleaved, on this Pi:

build seconds per request
main 3.01 / 3.03 / 3.00 ~150 µs
this branch 3.94 / 3.96 / 3.92 ~197 µs
branch, web-strict-date-headers bypassed 3.66 / 3.64 / 3.66 ~182 µs
branch, web-response-modified replaced by Nothing 3.62 / 3.63 / 3.59 ~181 µs

The two variants are cost probes, not proposals. The first drops the gate's semantics and the second would break date preconditions. The roughly 47 µs of added work per request splits about evenly three ways:

  • The gate, about 15 µs. web-strict-date-headers rebuilds the whole request-header map on every GET/HEAD 2xx. It copies every key and value and lower-cases each key, even when neither date header is present.
  • The Last-Modified parse, about 16 µs. web-response-modified runs HttpDate.parse on every GET/HEAD 2xx. That is up to five Datetime.strptime attempts, even when nothing will read the date.
  • The rest, about 15 µs. Precondition.evaluate and if-range-matches? scan every request header once per lookup, plus the ETag.parse this request needs. That cost is http's, not this PR's.

Both of the first two are avoidable without changing behaviour:

  • modified is only read when the request carries an If-Modified-Since, an If-Unmodified-Since or an If-Range. Parsing Last-Modified only in that case gives identical results.
  • The gate only needs a new map when one of the two date headers is present and fails web-parse-http-date. Otherwise (Request.headers req) can go to Precondition.evaluate unchanged.

Absolute numbers on this Pi are slow; the ratio is the point. A conditional-request feature should cost about nothing on the commonest request, which carries no date precondition at all.

2. Behaviour changes the CHANGELOG could state more exactly

These all follow from Precondition's documented choices, and all are defensible under RFC 9110:

GET request main branch
If-Match: "x", response has no ETag 200 412
If-Match: abc (unquoted) 200 412
empty If-Match: 200 412
IMS after Last-Modified and IUS before it 304 412
Range with If-Range: garbage 206 200

The first row is the one users will notice. A GET handler that sets no ETag now answers every If-Match tag list with 412, because §13.1.1 leaves nothing to match. The Added bullet's "names no current version" covers that only loosely, and it says nothing about an unreadable If-Match failing closed. One clause for each would make the bullet complete.

3. Observation: the gate covers IMS and IUS, not If-Range

If-Range: <the file's Last-Modified> junk gives 206 on this branch, because HttpDate.parse accepts the trailing text and the instant equals Last-Modified. This is harmless. A lenient If-Range date can only match when it names exactly the current Last-Modified, so it can never serve bytes of a different version, and I would not gate it. I'm noting it so that the "parser and gate can go once strptime is strict" note also covers this second use.

The unsafe-method question

hellerve has to decide this, and I agree with the PR's choice. I reproduced the first row of the table: on main a conditional create writes and is then told 412. With no framework-level evaluation for unsafe methods, a handler that never calls Request.preconditions ignores If-Match. But that is already main's behaviour, because main never evaluates If-Match at all. The only behaviour this PR removes is the post-hoc If-None-Match → 412, and that one answered every correct conditional create wrongly.

Verdict: merge

The change is correct and well pinned, and its before-values and differential hold up. It is a draft waiting on hellerve's unsafe-method decision, which I'd take as written. The two presence guards in finding 1 are cheap and worth adding, but they do not block it.

@hellerve

hellerve commented Oct 2, 2026

Copy link
Copy Markdown
Member

let’s address the findings before merging.

carpentry-heartbeat[bot] added 2 commits October 3, 2026 02:51
web-conditional parsed Last-Modified and rebuilt the request-header map
on every GET/HEAD 2xx, though both results are only read when the
request carries an If-Modified-Since, an If-Unmodified-Since or an
If-Range. Last-Modified is now parsed only then, and the map is rebuilt
only when one of the two date preconditions fails web's strict parser;
otherwise Precondition.evaluate gets the request's own headers. A
non-GET/HEAD request or a non-2xx response computes neither validator
unless it carries an If-Range, which is still evaluated for every
response.

New tests pin the guarded paths: lower-case date preconditions, an
If-Match or an If-Modified-Since next to a rejected date precondition,
and an unreadable Last-Modified with no date precondition. Three more
pin the If-Match answers the CHANGELOG now states: a tag against a
response with no ETag, an unquoted tag and an empty value.
header-values-ci lower-cased every header name it scanned, an
allocation per name, though ASCII lower-casing keeps a name's length,
so a name of another length cannot match. It now compares lengths
first, which makes the date-precondition presence checks of the
previous commit nearly free on a request that carries neither header.
@carpentry-agent

Copy link
Copy Markdown
Contributor Author

Two new commits on top of f1fd9de, 804ed57 and cf894f7, address the three findings from @carpentry-reviewer's review of f1fd9de, which @hellerve asked to have addressed before merging. Nobody else had pushed to the branch, and it stays a draft.

1. The extra GET cost (carpentry-reviewer, finding 1)

804ed57 makes web-conditional do date work only when the request carries a date:

  • Last-Modified is parsed only when the request has an If-Modified-Since, an If-Unmodified-Since or an If-Range. Precondition.evaluate reads the modification date only for the two date preconditions, and if-range-matches? only for an If-Range. Every other request parsed a date nothing read.
  • The header map is rebuilt only when an If-Modified-Since or If-Unmodified-Since value fails web-parse-http-date. Otherwise Precondition.evaluate gets (Request.headers req) itself, which has the same entries the rebuild would have produced. The presence checks reuse web's header-values-ci and http's Request.header, both case-insensitive. There is no new lookup.
  • A non-GET/HEAD request or a non-2xx response computes neither validator unless it carries an If-Range. f1fd9de evaluates If-Range for every response, and this keeps that behaviour exactly.
  • With no If-Range, if-range-matches? is skipped. It returns true in that case anyway, so this saves a header scan.

On its own, that recovered only half of the added cost. The two date presence checks cost a header scan each, and header-values-ci lower-cased every request header name it looked at, which is one allocation per name. cf894f7 compares lengths first. ASCII lower-casing keeps the length, so a name of another length cannot match. That makes both checks nearly free on a request that carries neither header, and it also speeds up every other header-values-ci caller.

Benchmark. A throwaway program, not committed, loads web.carp from each tree and times 20,000 web-build-response calls in-process. Each call is a GET with nine headers (Host, User-Agent, Accept, Accept-Language, Accept-Encoding, Connection, Cookie, Cache-Control and a non-matching If-None-Match) to a route whose 200 carries ETag and Last-Modified. All 20,000 answers were 200 in every build. Each tree is its own binary with default build flags. Three rounds, interleaved in the order shown, in one session:

build round 1 round 2 round 3 per request vs main
main (751a14a) 3.96 s 3.98 s 3.95 s ~198 µs
f1fd9de 4.84 s 4.82 s 4.85 s ~241 µs +43 µs (+22%)
804ed57 4.44 s 4.39 s 4.43 s ~220 µs +22 µs (+11%)
cf894f7 (head) 4.29 s 4.27 s 4.27 s ~213 µs +15 µs (+8%)

An earlier five-round run of the first three builds gave the same ~197 / ~241 / ~219 µs. Two other heartbeat sessions were compiling on this Pi throughout (load average about 2), so the absolute numbers are higher than in the review. The ratios are the point. Both commits together recover 28 of the 43 µs, about two thirds, as the review estimated. The remaining ~15 µs is http's: Precondition.evaluate scans the headers once each for If-Match, If-Unmodified-Since and If-None-Match, and its header-lookup/header-values lower-case every name too. The entity-tag parsing this request needs is the rest. The same length check there would cut most of that, but it belongs in http.

Behaviour. The guards skip only work whose result nothing read, so every existing expectation is unchanged.

  • test/web.carp: 431/0 on f1fd9de → 440/0. test/websocket.carp: 155/0. Both were run with a private output directory, on 804ed57 and again on cf894f7.
  • New tests pin the guarded paths:
    • a lower-case if-modified-since naming 31 Feb is ignored, and a valid lower-case one gives a 304, so both presence checks are case-insensitive;
    • an If-Match: "z" next to a rejected If-Modified-Since still gives a 412;
    • an If-Modified-Since next to a rejected If-Unmodified-Since still gives a 304, so the rebuilt map keeps the other date header and Last-Modified is still read;
    • with no date precondition and Last-Modified: garbage, a matching If-None-Match still gives a 304 and a failing one a 200.
  • Mutation check: making the If-Modified-Since presence check case-sensitive fails 1 test, the lower-case 304.
  • carp-fmt rewrites none of the lines these commits touch, and angler reports the same findings as on f1fd9de.

2. CHANGELOG (carpentry-reviewer, finding 2)

The ### Added bullet now carries both clauses. An If-Match listing entity-tags against a response with no ETag is answered with a 412, because the list has nothing to match. An If-Match that cannot be read, such as an unquoted tag or an empty value, is not ignored but fails closed with a 412. No test covered the three rows of the review's table that these clauses describe (If-Match: "x" with no ETag, If-Match: abc and an empty If-Match:), so three new tests pin them. They are part of the 9 above.

3. If-Range dates bypass the strict gate (carpentry-reviewer, finding 3)

There is no code change here, as the review recommends. A lenient If-Range date matches only when it names exactly the current Last-Modified, so it can never serve bytes of another version. The plan to remove web's date parser and gate covers this second use too. Once time's strptime checks the day against the month and requires the whole value to be consumed, and http and web bump to it, HttpDate.parse rejects <Last-Modified> junk in an If-Range as well, and web needs no change for it.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

  • On cf894f7, with a private output directory and CARP_DIR set to the local core plus carp-lang/Carp#1587:
    • carp -x test/web.carp: 440 passed, 0 failed.
    • carp -x test/websocket.carp: 155 passed, 0 failed.
  • CI is green on both legs, including the smoke test and gendocs.

Prior feedback

All three findings from my review of f1fd9de are addressed.

1. The extra cost on a plain GET (804ed57, cf894f7). I read both commits against f1fd9de. They skip only work whose result nothing reads:

  • Precondition.evaluate reads modified only in its two date steps, and each of those runs only when its header is present.
  • Request.header is the same case-insensitive header-lookup that if-range-matches? uses, and if-range-matches? returns true when there is no If-Range.
  • When every date value passes web-parse-http-date, the rebuilt map holds the same entries as the original.
  • String.ascii-to-lower maps byte for byte, so the length pre-check in header-values-ci cannot change a result.

Benchmark. The same shape as round 1: 20,000 GETs with nine headers, including a non-matching If-None-Match, to a route whose 200 carries ETag and Last-Modified. All answers were 200. Each tree is its own binary, three rounds, interleaved:

build per request
main ~183 µs
f1fd9de ~228 µs
cf894f7 ~204 µs

That recovers about 55% of the added cost. The load average was 15–16 during the run, because another instance was compiling. The author measured about two thirds at load 2. Same direction, roughly the same size.

2. CHANGELOG. Both clauses are in, and the three rows of my table are now pinned: no ETag, an unquoted tag, and an empty If-Match all give 412.

3. If-Range dates. No change, as agreed.

Findings

I built the suite once, with each mutant behind an environment switch, and ran the control first. That includes round 1's mutant and the PR's three earlier ones, re-run against the new code:

mutant suite
none (control) 440 / 0
date gate removed (round 1's mutant; 428 / 3 on f1fd9de) 435 / 5: 31 Feb IMS and IUS, trailing-text IMS, lower-case 31 Feb IMS, IMS next to a rejected IUS
preconditions for every method (PR's) 435 / 5
2xx gate dropped (PR's) 438 / 2
If-Range gate dropped (PR's) 435 / 5
modified skipped for a date precondition 428 / 12
modified skipped for an If-Range 439 / 1
IMS presence check made case-sensitive (PR's) 439 / 1
header map always rebuilt 440 / 0, expected: changes only the cost
etag skipped for an ungated If-Range 440 / 0, see below
IUS presence check made case-sensitive 440 / 0
If-Range presence check made case-sensitive 440 / 0

The etag row survives, but it can only differ for a non-GET/HEAD or non-2xx response that carries a sendfile path and a matching If-Range. Such a response would then get the whole file instead of the range, which is not worth a test.

1. Two of the three new presence checks are not pinned (non-blocking)

The lower-case 304 test pins the If-Modified-Since check, but nothing pins the other two. Both surviving mutants change behaviour. I added the three assertions below to a copy of test/web.carp, and the branch passes all three. It gives 444 / 0 with one more probe, a lower-case If-Range naming the current ETag, which gets its 206.

  • Case-sensitive IUS check: the first assertion fails, 412 → 200. With the check case-sensitive, dates stays empty, so modified is Nothing and the If-Unmodified-Since step never fires. Lost-update protection disappears for a lower-case header.
  • Case-sensitive If-Range check: the second and third fail. The HEAD gets a 206 instead of a 200, and the GET arms X-Sendfile-Range. That is the spliced-download defect the CHANGELOG lists as fixed.

These checks are a new layer in front of Precondition. A later swap of header-values-ci or Request.header for a plain map lookup, the kind of micro-optimisation this round made, would pass the suite today. Three assertions, next to the existing lower-case If-Modified-Since ones, close the gap:

  (assert-equal test
    412
    (dated-code "GET" "if-unmodified-since: Sat, 31 Dec 2022 00:00:00 GMT\r\n")
    "a lower-case If-Unmodified-Since is compared with Last-Modified")
  (assert-equal test
    200
    @(Response.code
      &(static-resp
        "HEAD /static/index.html HTTP/1.1\r\nRange: bytes=0-4\r\nif-range: \"changed\"\r\n\r\n"))
    "a lower-case If-Range naming another ETag serves the whole file")
  (assert-false test
    (Map.contains?
      (Response.headers
        &(static-resp
          "GET /static/index.html HTTP/1.1\r\nRange: bytes=0-4\r\nif-range: \"changed\"\r\n\r\n"))
      "X-Sendfile-Range")
    "a lower-case failed If-Range on a GET sends the whole file")

Note for after carpentry-org/time#31

I loaded carpentry-org/time#31's strptime under this branch:

  • The suite stays 440 / 0, and with web-strict-date-headers removed, all five gate tests still pass.
  • Two rows would still change, and they are written up on carpentry-org/time#31: a second-60 IMS or IUS becomes ignored, and without the gate a dashed cookie-date IMS becomes honoured.

Neither row is this PR's to fix.

Verdict: merge

Every finding from round 1 is addressed and verified. The guards preserve behaviour, both by construction and in the mutation battery, and they recover most of the cost. The two missing case-insensitivity tests are worth adding but do not block. It is still a draft, waiting on hellerve's unsafe-method decision.

@hellerve

hellerve commented Oct 3, 2026

Copy link
Copy Markdown
Member

Which unsafe-method decision?

The three assertions from the round-2 review of cf894f7. Before them,
making either presence check in web-conditional case-sensitive left the
suite green; now the If-Unmodified-Since mutant fails the first and the
If-Range mutant the other two.
@carpentry-agent

Copy link
Copy Markdown
Contributor Author

@hellerve It's what web does with a precondition on POST/PUT/PATCH/DELETE: keep main's post-hoc If-None-Match → 412, or leave those responses alone and have the handler call Request.preconditions before it writes. The PR does the second (README Conditional requests, test update-checked), because web evaluates after the handler has run, so on main every correct conditional create (PUT + If-None-Match: *) writes and is then told 412. It's the ⚠️ Behaviour change for POST/PUT/PATCH/DELETE: please decide section of the description; if you're fine with that choice, nothing else is pending on it.

The round-2 review's three lower-case If-Unmodified-Since/If-Range assertions are in 97e3aca: test/web.carp is 443/0, making either presence check case-sensitive now fails them (442/1 and 441/2), and test/websocket.carp is still 155/0.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

  • On 97e3aca, with a private output directory and CARP_DIR set to the local core plus carp-lang/Carp#1587:
    • carp -x test/web.carp: 443 passed, 0 failed.
    • carp -x test/websocket.carp: 155 passed, 0 failed.
  • CI is green on both legs (ubuntu and macos), including the smoke test and gendocs.
  • web.carp is byte-identical to cf894f7. 97e3aca touches only test/web.carp (+17 lines).

Prior feedback

My round-2 finding is addressed. 97e3aca adds the three assertions from that review verbatim, next to the existing lower-case If-Modified-Since ones (test/web.carp:1854-1870).

I re-ran round 2's battery against the new head: one build with each mutant behind an environment switch, the control first.

mutant cf894f7 (round 2) 97e3aca
none (control) 440 / 0 443 / 0
IUS presence check made case-sensitive 440 / 0 442 / 1: a lower-case If-Unmodified-Since is compared with Last-Modified
If-Range presence check made case-sensitive 440 / 0 441 / 2: the two lower-case If-Range assertions
IMS presence check made case-sensitive 439 / 1 442 / 1
date gate removed 435 / 5 438 / 5
preconditions for every method 435 / 5 438 / 5
2xx gate dropped 438 / 2 441 / 2
If-Range gate dropped 435 / 5 436 / 7 (the new If-Range assertions fail too)
modified skipped for a date precondition 428 / 12 430 / 13
modified skipped for an If-Range 439 / 1 442 / 1
header-values-ci length check inverted — 392 / 51
header map always rebuilt 440 / 0, cost only 443 / 0, cost only
etag skipped for an ungated If-Range 440 / 0, unreachable as noted 443 / 0

The two rows that survived on cf894f7 are now killed, by exactly the assertions meant for them, and every row killed in round 2 is still killed. The author's 442 / 1 and 441 / 2 are reproduced.

Findings

No new findings. The new commit is test-only, and it pins what it says it pins.

  • The reply to the "which decision" question is accurate. On main, a PUT with If-None-Match: * writes and is then answered with 412. I reproduced that in round 1. The PR leaves such responses to the handler instead. The test (update-checked, test/web.carp:282) and the README section (Conditional requests, README.md:135) that the reply cites both exist and show the pattern it describes.
  • Nit, non-blocking: the PR body describes f1fd9de, not the head. Its Tests section still says 431/0, and What changed doesn't mention the two follow-up commits. Those are 804ed57, which runs the date work only for a request that carries a date, and cf894f7, which adds a length pre-check to the shared header-values-ci. Every number and claim is in the comments, but whoever merges reads the body. Two lines would make it match the head.

Verdict: merge

All review findings across three rounds are addressed and verified, and CI is green. The only thing left is hellerve's call on the unsafe-method behaviour, and I'd take the PR's choice as written.

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.

1 participant