Normalize cookie flag semantics

[medz]

Summary by CodeRabbit

• Documentation

  • Added Flag Semantics section explaining HttpOnly, Secure, and Partitioned are two-state flags and that omitting a flag is equivalent to false.

• Refactor

  • Cookie flags (HttpOnly, Secure, Partitioned) now behave as non-nullable booleans with default false; parsing/serialization and equality reflect this.

• Tests

  • Enhanced tests for parsing, flag equivalence (omitted vs explicit false), and Set-Cookie splitting.

[medz]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

📝 Walkthrough

Walkthrough

Converted Cookie flag fields (httpOnly, secure, partitioned) from nullable to non-nullable booleans with default false; removed corresponding enum entries; updated parsing, validation, serialization, equality, and tests; added docs describing flag semantics and parsing behavior.

Changes

Cohort / File(s)	Summary
Documentation CHANGELOG.md, README.md	Added Unreleased notes and a "Flag Semantics" section: HttpOnly, Secure, and Partitioned are two-state flags; omitting a flag is equivalent to explicit false.
Core Implementation lib/src/cookie.dart	Made httpOnly, secure, partitioned non-nullable bool with default false; removed those entries from CookieNullableField; updated constructor, copyWith, parsing (parseFlagValue usage), validation (SameSite-none checks), serialization, equality, and hashCode.
Tests test/copy_with_test.dart, test/from_string_test.dart	Updated copyWith/clear behavior tests to drop clearing of removed nullable fields; added equality tests ensuring omitted vs. explicit-false flags are equivalent; expanded parsing tests (quoted values, equals signs, unknown attributes, last-wins behavior, repeated attributes, combined headers splitting).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibbled at flags—three little lights,
From maybe to false, they settled their plights.
No more null-holes in cookies I keep,
Now solid and simple—sleep after sleep. 🥕✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Normalize cookie flag semantics' accurately summarizes the main change: converting httpOnly, secure, and partitioned fields from nullable to non-nullable two-state flags with false defaults.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/normalize-cookie-flags

📝 Coding Plan

• Generate coding plan for human review comments

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

test/from_string_test.dart (1)

82-90: Please lock down the =false form too.

Lines 82-90 already establish that non-standard values like 1 and true still enable the flag. With the new two-state wording, Secure=false, HttpOnly=false, and Partitioned=false are the inputs most likely to surprise users, so I'd add that case here as well to keep the wire-format contract unambiguous.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/from_string_test.dart` around lines 82 - 90, Add assertions to the
existing test "should treat valueless flags with non-standard values as enabled"
(which calls Cookie.fromString) to also verify that explicit "=false" disables
the flags; specifically, parse a cookie string like 'sid=abc; Secure=false;
HttpOnly=false; Partitioned=false' using Cookie.fromString and assert that
cookie.secure, cookie.httpOnly, and cookie.partitioned are all false so the
parser treats the "=false" form as the disabled state.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CHANGELOG.md`:
- Around line 3-4: Update the changelog entry to explicitly document the
breaking API change: state that the properties CookieNullableField.httpOnly,
CookieNullableField.secure, and CookieNullableField.partitioned have been
removed (compile-time break) and describe the migration path (e.g., replace
usages with the new two-state flags on Cookie or set explicit false/omitted
semantics accordingly), so consumers know how to update their code; ensure the
wording appears near the existing lines about flag semantics to make the
relationship clear.

---

Nitpick comments:
In `@test/from_string_test.dart`:
- Around line 82-90: Add assertions to the existing test "should treat valueless
flags with non-standard values as enabled" (which calls Cookie.fromString) to
also verify that explicit "=false" disables the flags; specifically, parse a
cookie string like 'sid=abc; Secure=false; HttpOnly=false; Partitioned=false'
using Cookie.fromString and assert that cookie.secure, cookie.httpOnly, and
cookie.partitioned are all false so the parser treats the "=false" form as the
disabled state.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e7255ab4-a441-4165-abc1-01ea697f92b0

📥 Commits

Reviewing files that changed from the base of the PR and between d0bc286 and 1eadfdf.

📒 Files selected for processing (5)

• CHANGELOG.md
• README.md
• lib/src/cookie.dart
• test/copy_with_test.dart
• test/from_string_test.dart

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Document the clear API break explicitly.

Lines 3-4 describe the semantic shift, but the compile-time break most consumers will hit is that CookieNullableField.httpOnly, CookieNullableField.secure, and CookieNullableField.partitioned are gone. Please call that out here so the migration path is discoverable from the release notes.

✏️ Suggested changelog wording

-- make `httpOnly`, `secure`, and `partitioned` two-state flags with `false` defaults
-- treat omitted flags and explicit `false` as the same value semantics
+- BREAKING: make `httpOnly`, `secure`, and `partitioned` two-state flags with `false` defaults
+- BREAKING: treat omitted flags and explicit `false` as the same value semantics
+- BREAKING: remove `CookieNullableField.httpOnly`, `CookieNullableField.secure`, and
+  `CookieNullableField.partitioned`; `copyWith(clear: ...)` can no longer clear those flags

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@CHANGELOG.md` around lines 3 - 4, Update the changelog entry to explicitly
document the breaking API change: state that the properties
CookieNullableField.httpOnly, CookieNullableField.secure, and
CookieNullableField.partitioned have been removed (compile-time break) and
describe the migration path (e.g., replace usages with the new two-state flags
on Cookie or set explicit false/omitted semantics accordingly), so consumers
know how to update their code; ensure the wording appears near the existing
lines about flag semantics to make the relationship clear.

[coderabbitai]

🧹 Nitpick comments (1)

test/from_string_test.dart (1)

82-96: Consider adding test coverage for 0 and ?0 flag values.

The test covers 1, true, ?1, and false, but parseFlagValue also handles 0 and ?0 as false. Adding these cases would improve coverage.

🧪 Suggested additional test case

     final disabledCookie = Cookie.fromString(
       'sid=abc; Secure=false; HttpOnly=false; Partitioned=false',
     );
+    final disabledCookieAlt = Cookie.fromString(
+      'sid=abc; Secure=0; HttpOnly=?0; Partitioned=0',
+    );

     expect(enabledCookie.secure, isTrue);
     expect(enabledCookie.httpOnly, isTrue);
     expect(enabledCookie.partitioned, isTrue);
     expect(disabledCookie.secure, isFalse);
     expect(disabledCookie.httpOnly, isFalse);
     expect(disabledCookie.partitioned, isFalse);
+    expect(disabledCookieAlt.secure, isFalse);
+    expect(disabledCookieAlt.httpOnly, isFalse);
+    expect(disabledCookieAlt.partitioned, isFalse);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/from_string_test.dart` around lines 82 - 96, Add assertions to the
existing test 'should treat non-standard flag values predictably' to cover the
missing false-equivalent flag strings "0" and "?0": create Cookie.fromString
instances (e.g., a cookie with Secure=0; HttpOnly=?0; Partitioned=0 and another
mixing those forms) and assert their secure, httpOnly, and partitioned
properties are false so parseFlagValue's handling of "0" and "?0" is validated;
update the test around the existing enabledCookie/disabledCookie cases
(referencing Cookie.fromString and the properties secure, httpOnly, partitioned)
to include these additional negative cases.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/from_string_test.dart`:
- Around line 82-96: Add assertions to the existing test 'should treat
non-standard flag values predictably' to cover the missing false-equivalent flag
strings "0" and "?0": create Cookie.fromString instances (e.g., a cookie with
Secure=0; HttpOnly=?0; Partitioned=0 and another mixing those forms) and assert
their secure, httpOnly, and partitioned properties are false so parseFlagValue's
handling of "0" and "?0" is validated; update the test around the existing
enabledCookie/disabledCookie cases (referencing Cookie.fromString and the
properties secure, httpOnly, partitioned) to include these additional negative
cases.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d4d4eb8f-f145-4803-a1eb-74f41aa7896d

📥 Commits

Reviewing files that changed from the base of the PR and between 1eadfdf and 49f644e.

📒 Files selected for processing (3)

• CHANGELOG.md
• lib/src/cookie.dart
• test/from_string_test.dart