fix: align RE_COOKIE_OCTET with RFC 6265 §4.1.1 to exclude backslash by kawasima · Pull Request #82 · enkan/enkan

kawasima · 2026-03-08T04:12:35Z

Summary

Rewrites RE_COOKIE_OCTET in CookiesMiddleware using explicit hex ranges (\x21\x23-\x2B\x2D-\x3A\x3C-\x5B\x5D-\x7E) that exactly match the RFC 6265 §4.1.1 definition
Removes backslash (%x5C) from the accepted set — it was inadvertently included via \\[\\]\\^ in the old character class
Adds two tests: RFC-valid octets at range boundaries still parse correctly; backslash is no longer accepted as a cookie-octet value

Test plan

CookiesMiddlewareTest — all 4 tests pass
Existing parse test continues to pass with URL-encoded values

Closes #74

🤖 Generated with Claude Code

The previous character class included backslash (%x5C) via \\[\\]\\^, which is not a valid cookie-octet per RFC 6265 §4.1.1. Replace the ad-hoc character class with explicit hex ranges that exactly match the RFC definition: %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E. Add tests confirming RFC-valid octets still parse and backslash is no longer accepted as a cookie-octet character. Closes #74 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR tightens cookie parsing in CookiesMiddleware to be strictly RFC 6265 §4.1.1 compliant by updating the cookie-octet regex so it no longer permits backslash characters in cookie values.

Changes:

Rewrite RE_COOKIE_OCTET using explicit hex ranges that match RFC 6265’s cookie-octet definition and exclude %x5C (backslash).
Add tests covering RFC-valid cookie octets and ensuring backslash is not accepted in cookie values.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
enkan-web/src/main/java/enkan/middleware/CookiesMiddleware.java	Updates `RE_COOKIE_OCTET` to RFC 6265 hex ranges, excluding backslash from acceptable cookie octets.
enkan-web/src/test/java/enkan/middleware/CookiesMiddlewareTest.java	Adds tests for allowed octets and verifies backslash is not consumed as part of a cookie value.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-08T04:15:58Z

+        MiddlewareChain<HttpRequest, HttpResponse, ?, ?> chain = new DefaultMiddlewareChain<>(new AnyPredicate<>(), null,
+                (Endpoint<HttpRequest, HttpResponse>) req -> {
+                    assertThat(req.getCookies()).containsKey("A");
+                    return HttpResponse.of("ok");
+                });
+        request.getHeaders().put("Cookie", "A=!#%2B-:%3C%5B%5D~");


parsesRfcValidCookieOctets percent-encodes several of the “boundary” characters it claims to be validating (+, <, [, ]). Since the regex is applied to the raw header before formDecodeStr, this test currently only proves that %, hex digits, etc. are accepted and that the cookie key exists; it wouldn’t fail if </[/] were actually rejected. Consider using the raw characters in the header where possible and assert the parsed cookie value equals the expected decoded string so the test detects truncation/partial matches.

Suggested change

MiddlewareChain<HttpRequest, HttpResponse, ?, ?> chain = new DefaultMiddlewareChain<>(new AnyPredicate<>(), null,

(Endpoint<HttpRequest, HttpResponse>) req -> {

assertThat(req.getCookies()).containsKey("A");

return HttpResponse.of("ok");

});

request.getHeaders().put("Cookie", "A=!#%2B-:%3C%5B%5D~");

final String expectedValue = "!#+-:<[]~";

MiddlewareChain<HttpRequest, HttpResponse, ?, ?> chain = new DefaultMiddlewareChain<>(new AnyPredicate<>(), null,

(Endpoint<HttpRequest, HttpResponse>) req -> {

Cookie cookie = req.getCookies().get("A");

assertThat(cookie).isNotNull();

assertThat(cookie.getValue()).isEqualTo(expectedValue);

return HttpResponse.of("ok");

});

request.getHeaders().put("Cookie", "A=!#+-:<[]~");

… test The previous test percent-encoded boundary chars before putting them in the header, which only validated that percent-encoding itself is accepted, not the raw octets. Use literal chars and assert the full decoded value so the test actually verifies each boundary character is accepted by RE_COOKIE_OCTET. Replace '+' (%x2B, RFC-valid) with '%' (%x25) since formDecodeStr decodes '+' as a space in form-encoded strings. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

kawasima

Fixed. The test now uses raw cookie-octet chars directly in the header value and asserts the full decoded value, so any character in the set that fails to match would cause a truncated/wrong value assertion failure.

Note: (%x2B) is RFC-valid as a cookie-octet but replaced with (%x25) in the test because decodes as a space (form-encoding convention). The regex itself correctly accepts .

kawasima

Fixed. The test now uses raw cookie-octet chars directly in the header value and asserts the full decoded value, so any character in the set that fails to match would cause a truncated/wrong value assertion failure.

Note: + (%x2B) is RFC-valid as a cookie-octet but replaced with % (%x25) in the test because formDecodeStr decodes + as a space (form-encoding convention). The regex itself correctly accepts +.

Add a note that the malformed-input rule applies to tests as well: verify that each test actually exercises the code path it claims to test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-08T04:27:03Z

+    // RFC 6265 §4.1.1: cookie-octet = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
+    // Backslash (%x5C) and double-quote (%x22) are explicitly excluded.
+    private static final Pattern RE_COOKIE_OCTET = Pattern.compile("[\\x21\\x23-\\x2B\\x2D-\\x3A\\x3C-\\x5B\\x5D-\\x7E]");


The old RE_COOKIE_OCTET character class did not appear to include a literal backslash (it only used backslashes to escape [, ], ^, {, |, } inside the class). As a result, this change may be a no-op with respect to excluding \\, and the new test might already have passed before this PR. Consider adjusting the PR description/rationale (and/or adding a regression test that would fail with the previous regex if there is a concrete behavior difference you’re targeting).

Copilot · 2026-03-08T04:27:04Z

+        // RFC 6265 §4.1.1: spot-check boundary chars of each allowed range using raw octets.
+        // %x21=!, %x23=#, %x25=%, %x2D=-, %x3A=:, %x3C=<, %x5B=[, %x5D=], %x7E=~
+        // Note: %x2B (+) is RFC-valid but decoded as space by formDecodeStr, so % is used instead.
+        final String expectedValue = "!#%-:<[]~";


This test value includes a raw %, which causes URLDecoder.decode (via formDecodeStr) to throw IllegalArgumentException and fall back to returning the original string unchanged. That means the test is exercising the decoder error/fallback path rather than the normal decoding path, making it less focused/robust. Consider using only characters that don’t have special meaning to URLDecoder (or encode % as %25 so decoding succeeds) while still covering the intended cookie-octet ranges.

Copilot · 2026-03-08T04:27:04Z

+    void rejectsBackslashInCookieValue() {
+        // Backslash (%x5C) is excluded by RFC 6265 §4.1.1 cookie-octet definition.
+        // The backslash is not consumed as a cookie-octet; RE_COOKIE_VALUE matches the
+        // empty string before it, so the cookie is parsed with an empty value.
+        MiddlewareChain<HttpRequest, HttpResponse, ?, ?> chain = new DefaultMiddlewareChain<>(new AnyPredicate<>(), null,
+                (Endpoint<HttpRequest, HttpResponse>) req -> {
+                    Cookie bad = req.getCookies().get("BAD");
+                    assertThat(bad).isNotNull();
+                    assertThat(bad.getValue()).isEmpty();


The test name says it “rejects” backslash, but the asserted behavior is that the cookie is still parsed and the value becomes the empty string. Consider renaming the test to reflect the actual behavior being asserted (e.g., that backslash is not consumed as part of the value).

…sh test - Replace % and + in parsesRfcValidCookieOctets with chars that have no special meaning for form-decoding (% triggers URLDecoder error path, + is decoded as space); use !#-:<[]~ which pass through unchanged - Rename rejectsBackslashInCookieValue -> backslashIsNotConsumedAsCookieOctet to reflect actual behavior: backslash is not consumed as a cookie-octet, so the value becomes the empty string rather than the key being absent Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

kawasima

Thank you for the careful analysis.

Backslash was not in old regex (comment #2): Confirmed — verified by testing the old pattern, backslash was already rejected. The PR is a readability/conformance change (explicit hex ranges matching RFC 6265 §4.1.1 literally) rather than a behavioral fix. Updated the commit message to reflect this.

Test fixes (comments #1, #3, #4):

parsesRfcValidCookieOctets: now uses raw chars !#-:<[]~ (one from each RFC range boundary) and asserts cookie.getValue() equals that exact string. % and + are excluded since formDecodeStr gives them special treatment.
Renamed rejectsBackslashInCookieValue → backslashIsNotConsumedAsCookieOctet to reflect the actual observed behavior (empty value, not absent key).

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-08T04:38:04Z

+    // Backslash (%x5C) and double-quote (%x22) are explicitly excluded.
+    private static final Pattern RE_COOKIE_OCTET = Pattern.compile("[\\x21\\x23-\\x2B\\x2D-\\x3A\\x3C-\\x5B\\x5D-\\x7E]");
    private static final Pattern RE_COOKIE_VALUE = Pattern.compile("\"" + RE_COOKIE_OCTET.pattern() +  "*\"|" + RE_COOKIE_OCTET.pattern() + "*");
    private static final Pattern RE_COOKIE = Pattern.compile("\\s*(" + RE_TOKEN + ")=(" + RE_COOKIE_VALUE.pattern() + ")\\s*[;,]?");


RE_COOKIE/RE_COOKIE_VALUE currently allows a zero-length cookie value match (*) without requiring a delimiter or end-of-header afterwards. With the new RE_COOKIE_OCTET excluding backslash, a header like BAD=\ will still be parsed as a cookie named BAD with an empty value (the match stops at BAD= and ignores the following invalid char). If the intent is to reject malformed cookie-pairs containing non cookie-octets, update the cookie-pair regex to require that the value is immediately followed by a valid terminator (,/; or end-of-string) rather than any character.

Suggested change

private static final Pattern RE_COOKIE = Pattern.compile("\\s*(" + RE_TOKEN + ")=(" + RE_COOKIE_VALUE.pattern() + ")\\s*[;,]?");

private static final Pattern RE_COOKIE = Pattern.compile("\\s*(" + RE_TOKEN + ")=(" + RE_COOKIE_VALUE.pattern() + ")\\s*(?:[;,]|$)");

Copilot · 2026-03-08T04:38:04Z

+    void backslashIsNotConsumedAsCookieOctet() {
+        // Backslash (%x5C) is excluded by RFC 6265 §4.1.1 cookie-octet definition.
+        // The backslash is not consumed as a cookie-octet; RE_COOKIE_VALUE matches the
+        // empty string before it, so the cookie value is empty.
+        MiddlewareChain<HttpRequest, HttpResponse, ?, ?> chain = new DefaultMiddlewareChain<>(new AnyPredicate<>(), null,
+                (Endpoint<HttpRequest, HttpResponse>) req -> {
+                    Cookie bad = req.getCookies().get("BAD");
+                    assertThat(bad).isNotNull();
+                    assertThat(bad.getValue()).isEmpty();
+                    return HttpResponse.of("ok");
+                });
+        request.getHeaders().put("Cookie", "BAD=\\");
+        middleware.handle(request, chain);


This test asserts that a backslash in the Cookie header produces a cookie with an empty value. That outcome is a side effect of RE_COOKIE_VALUE matching the empty string (via *) and RE_COOKIE not requiring a delimiter/end-of-header after the value, so the parser silently truncates at BAD= rather than rejecting the malformed cookie-pair. If the goal is “backslash is not accepted as cookie-octet”, consider asserting that BAD is not present (or tightening the cookie-pair regex so non cookie-octets prevent a match) instead of locking in the truncation behavior.

…n-octets Change RE_COOKIE from [;,]? (optional delimiter) to (?:[;,]|$) so that a cookie-pair like BAD=\ — where backslash follows the value — does not match at all, rather than silently producing an empty-value cookie. Update backslashIsNotConsumedAsCookieOctet to assert the cookie is absent. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

kawasima

Addressed both new comments.

Backslash no longer produces empty-value cookie: Changed RE_COOKIE from [;,]? (optional delimiter) to (?:[;,]|$), requiring the value to be followed by a valid delimiter or end-of-input. BAD=\ now produces no match at all since \ is neither a cookie-octet, ;, ,, nor end-of-string. Test updated to assert the key is absent.

Behavioral difference vs old code: The old regex also had [;,]? so BAD=\ would have produced BAD="" (empty value) with both the old and new RE_COOKIE_OCTET. The RE_COOKIE change is the substantive behavioral fix here.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-08T04:43:23Z

+    void backslashIsNotConsumedAsCookieOctet() {
+        // Backslash (%x5C) is not a valid cookie-octet (RFC 6265 §4.1.1).
+        // RE_COOKIE requires the value to be followed by a delimiter or end-of-input,
+        // so "BAD=\" does not match and the cookie is not parsed at all.


The explanatory comment shows BAD=\", which reads as a backslash followed by a quote. The actual header used in the test is a trailing backslash (BAD=\\ in Java source → BAD=\ at runtime). Update the comment to match the test input so the failure mode is unambiguous.

Suggested change

// so "BAD=\" does not match and the cookie is not parsed at all.

// so BAD=\ (a trailing backslash) does not match and the cookie is not parsed at all.

Copilot · 2026-03-08T04:43:24Z

+    // RFC 6265 §4.1.1: cookie-octet = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
+    // Backslash (%x5C) and double-quote (%x22) are explicitly excluded.
+    private static final Pattern RE_COOKIE_OCTET = Pattern.compile("[\\x21\\x23-\\x2B\\x2D-\\x3A\\x3C-\\x5B\\x5D-\\x7E]");
    private static final Pattern RE_COOKIE_VALUE = Pattern.compile("\"" + RE_COOKIE_OCTET.pattern() +  "*\"|" + RE_COOKIE_OCTET.pattern() + "*");


There is an extra double-space in the string concatenation (pattern() + "*"). This is easy to miss when editing regexes; consider reducing it to a single space to avoid accidental formatting-only diffs later.

Suggested change

private static final Pattern RE_COOKIE_VALUE = Pattern.compile("\"" + RE_COOKIE_OCTET.pattern() + "*\"|" + RE_COOKIE_OCTET.pattern() + "*");

private static final Pattern RE_COOKIE_VALUE = Pattern.compile("\"" + RE_COOKIE_OCTET.pattern() + "*\"|" + RE_COOKIE_OCTET.pattern() + "*");

…mment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

kawasima

Fixed both:

Double-space in RE_COOKIE_VALUE removed
Test comment updated to BAD=\\ (a trailing backslash) to match the actual runtime input

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

kawasima requested a review from Copilot March 8, 2026 04:13

Copilot started reviewing on behalf of kawasima March 8, 2026 04:13

View session