Skip to content

Commit 65e08f6

Browse files
etrclaude
andcommitted
TASK-064: cosmetic cleanup — remove dead code, drop explicit append counts, update specs
1. Delete dead cookie_token_split struct and split_cookie_token helper (code-simplifier-iter1-1): these 27 lines were never called; the inline logic in parse_cookie_header was written independently. 2. Replace explicit byte-count literals in append calls with single- argument form (code-simplifier-iter1-3): out.append("; Secure", 8) → out.append("; Secure") etc. across append_time_attributes, append_target_attributes, and append_flag_attributes. The compiler derives the length at compile time; no runtime cost change. 3. Mark all five TASK-064 action items complete in TASK-064.md (housekeeper-iter1-1). 4. Add §3.5.1 Structured Cookie Type (API-CKY) to product_specs.md with 10 EARS requirements covering the cookie value type, injection guards, RFC 6265 rendering, parse semantics, deprecated shim policy, and acceptance criteria (housekeeper-iter1-2). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 647feee commit 65e08f6

3 files changed

Lines changed: 52 additions & 42 deletions

File tree

specs/product_specs.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,47 @@ The response hierarchy has eight subclasses (`string_response`, `file_response`,
184184

185185
---
186186

187+
### 3.5.1 Structured Cookie Type (API-CKY)
188+
189+
**Problem / outcome**
190+
The v1 `with_cookie(string, string)` / `get_cookie(string)` API stored cookies as opaque string blobs, making RFC 6265 attribute fields (Secure, HttpOnly, SameSite, Path, Domain, Expires, Max-Age) inaccessible to callers and allowing header-injection via unguarded semicolons in values. After TASK-064 the library exposes `httpserver::cookie`, a structured value type, alongside `http_response::with_cookie(cookie)` for responses and `http_request::get_cookies_parsed()` for requests. The string-blob path is retained as a `[[deprecated]]` shim for one transitional v2.0 release.
191+
192+
**In scope**
193+
- `httpserver::cookie` value type with fluent `with_name`, `with_value`, `with_domain`, `with_path`, `with_expires`, `with_max_age`, `with_secure`, `with_http_only`, `with_same_site` setters.
194+
- `enum class same_site_mode { unset, strict, lax, none }`.
195+
- `cookie::to_set_cookie_header()` renders a fully RFC 6265 §4.1 conformant `Set-Cookie` value.
196+
- `cookie::parse_cookie_header(string_view)` parses a `Cookie:` request header into a `std::vector<cookie>`, byte-transparent and lenient.
197+
- `http_response::with_cookie(cookie)` structured overload; legacy `with_cookie(string, string)` marked `[[deprecated]]`.
198+
- `http_request::get_cookies_parsed()` returns `const std::vector<cookie>&`, parsed once and cached.
199+
- Injection guard: CR, LF, NUL, and `;` are rejected in name, value, domain, and path at setter time (CWE-113).
200+
- SameSite=None auto-coerces `Secure` at render time (browser requirement per draft-west-cookie-incrementalism).
201+
- Transitional policy: legacy `get_cookie(string)`, `get_cookies()`, and `with_cookie(string, string)` are `[[deprecated]]` and compile with a diagnostic in v2.0.
202+
203+
**Out of scope**
204+
- Removing the deprecated string-blob path before v2.1.
205+
- Domain-format hostname validation beyond semicolon rejection.
206+
207+
**EARS Requirements**
208+
- `PRD-CKY-REQ-001` When a user constructs a `httpserver::cookie` then the system shall provide fluent `with_*` setters that return `cookie&` on lvalue and `cookie&&` on rvalue, mirroring the `http_response` fluent style.
209+
- `PRD-CKY-REQ-002` When a user calls `with_name`, `with_value`, `with_domain`, or `with_path` with a value containing CR, LF, NUL, or `;` then the system shall throw `std::invalid_argument`.
210+
- `PRD-CKY-REQ-003` When a user calls `with_name` with a value containing `=` or ASCII whitespace then the system shall throw `std::invalid_argument`.
211+
- `PRD-CKY-REQ-004` When a user calls `cookie::to_set_cookie_header()` then the system shall produce a string conforming to RFC 6265 §4.1 with attributes in the canonical order: Expires, Max-Age, Domain, Path, Secure, HttpOnly, SameSite.
212+
- `PRD-CKY-REQ-005` When a user calls `cookie::to_set_cookie_header()` with `same_site_mode::none` set and `with_secure(false)` then the system shall include `Secure` in the output (browser requirement).
213+
- `PRD-CKY-REQ-006` When a user calls `cookie::parse_cookie_header(s)` then the system shall return a `std::vector<cookie>` parsed from the `Cookie:` wire format, stripping outer DQUOTE pairs from values and tolerating arbitrary ASCII whitespace around tokens.
214+
- `PRD-CKY-REQ-007` When a user calls `http_response::with_cookie(cookie)` then the system shall append the structured cookie to the response's cookie list; `get_cookies_parsed()` shall reflect it without copying the cookie object.
215+
- `PRD-CKY-REQ-008` When a user calls `http_request::get_cookies_parsed()` then the system shall return a `const std::vector<cookie>&` backed by a parse-once cached representation; subsequent calls shall not allocate.
216+
- `PRD-CKY-REQ-009` When v2.0 ships then `http_response::with_cookie(string, string)`, `http_response::get_cookie(string_view)`, and `http_response::get_cookies()` shall be `[[deprecated]]` and shall not be removed until v2.1.
217+
- `PRD-CKY-REQ-010` When a user calls the legacy `with_cookie(string, string)` shim with a name or value that violates RFC 6265 cookie-name or cookie-value rules then the system shall throw `std::invalid_argument` before mutating any internal state.
218+
219+
**Acceptance criteria**
220+
- `http_response::string("body").with_cookie(cookie{}.with_name("sid").with_secure(true).with_same_site(same_site_mode::strict))` compiles and produces a single well-formed `Set-Cookie` value.
221+
- `http_request::get_cookies_parsed()` returns `const std::vector<cookie>&`; pointer identity is stable across unrelated mutations.
222+
- RFC 6265 round-trip examples pass (`cookie_render` test suite, cycles 2–6).
223+
- `cookie{}.with_path("/; Secure")` throws `std::invalid_argument`.
224+
- Deprecated string-blob path compiles with `-Wdeprecated-declarations` diagnostic.
225+
226+
---
227+
187228
### 3.6 Request Type Ergonomics (API-REQ)
188229

189230
**Problem / outcome**

specs/tasks/M7-v2-cleanup/TASK-064.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88
Replace the string-blob cookie surface on `http_response` with a structured `httpserver::cookie` value type carrying `name`, `value`, `domain`, `path`, `expires`, `max_age`, `secure`, `http_only`, `same_site`. The follow-up was explicitly deferred at `src/httpserver/http_response.hpp:304-313`.
99

1010
**Action Items:**
11-
- [ ] Design the `httpserver::cookie` value type in a new public header `src/httpserver/cookie.hpp`. Default-construct empty; provide fluent `with_*` setters mirroring `http_response`'s style. Include enum `same_site_mode { unset, strict, lax, none }`.
12-
- [ ] Add `http_response::with_cookie(cookie)` and `http_response::with_cookie(std::string name, std::string value)` overloads. Internally render to the `Set-Cookie` header per RFC 6265 §4.1.
13-
- [ ] Provide `http_request::get_cookies()` returning a structured view (parsed once, cached on the request impl per TASK-016 arena pattern).
14-
- [ ] Document migration: legacy string-blob path remains as a `[[deprecated]]` thin shim for one transitional release.
15-
- [ ] Add a unit test pinning round-trip parsing/rendering against RFC 6265 examples.
11+
- [x] Design the `httpserver::cookie` value type in a new public header `src/httpserver/cookie.hpp`. Default-construct empty; provide fluent `with_*` setters mirroring `http_response`'s style. Include enum `same_site_mode { unset, strict, lax, none }`.
12+
- [x] Add `http_response::with_cookie(cookie)` and `http_response::with_cookie(std::string name, std::string value)` overloads. Internally render to the `Set-Cookie` header per RFC 6265 §4.1.
13+
- [x] Provide `http_request::get_cookies()` returning a structured view (parsed once, cached on the request impl per TASK-016 arena pattern).
14+
- [x] Document migration: legacy string-blob path remains as a `[[deprecated]]` thin shim for one transitional release.
15+
- [x] Add a unit test pinning round-trip parsing/rendering against RFC 6265 examples.
1616

1717
**Dependencies:**
1818
- Blocked by: TASK-016 (arena), TASK-009 (http_response value type)

src/cookie.cpp

Lines changed: 6 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -163,37 +163,6 @@ std::string_view same_site_attribute_text(same_site_mode m) noexcept {
163163
return {};
164164
}
165165

166-
// Split a cookie-pair token into trimmed name + (DQUOTE-stripped)
167-
// value. Returns {has_value, name_view, value_view}.
168-
// - has_value=false: token had no `=` or had no name; caller skips.
169-
// - otherwise: name and value are byte-views into `tok`.
170-
struct cookie_token_split {
171-
bool ok;
172-
std::string_view name;
173-
std::string_view value;
174-
};
175-
176-
cookie_token_split split_cookie_token(std::string_view tok) noexcept {
177-
const std::size_t eq = tok.find('=');
178-
if (eq == std::string_view::npos || eq == 0) {
179-
return {false, {}, {}};
180-
}
181-
std::string_view name_sv = trim_ws(tok.substr(0, eq));
182-
std::string_view value_sv = tok.substr(eq + 1);
183-
184-
// Strip a single outer DQUOTE pair from the value (RFC 6265
185-
// §5.2 step 2 / common browser practice).
186-
if (value_sv.size() >= 2
187-
&& value_sv.front() == '"'
188-
&& value_sv.back() == '"') {
189-
value_sv = value_sv.substr(1, value_sv.size() - 2);
190-
}
191-
if (name_sv.empty()) {
192-
return {false, {}, {}};
193-
}
194-
return {true, name_sv, value_sv};
195-
}
196-
197166
} // namespace
198167

199168
// ----------------------------------------------------------------------
@@ -338,22 +307,22 @@ std::string cookie::to_set_cookie_header() const {
338307

339308
void cookie::append_time_attributes(std::string& out) const {
340309
if (expires_.has_value()) {
341-
out.append("; Expires=", 10);
310+
out.append("; Expires=");
342311
out.append(format_imf_fixdate(*expires_));
343312
}
344313
if (max_age_.has_value()) {
345-
out.append("; Max-Age=", 10);
314+
out.append("; Max-Age=");
346315
out.append(std::to_string(*max_age_));
347316
}
348317
}
349318

350319
void cookie::append_target_attributes(std::string& out) const {
351320
if (!domain_.empty()) {
352-
out.append("; Domain=", 9);
321+
out.append("; Domain=");
353322
out.append(domain_);
354323
}
355324
if (!path_.empty()) {
356-
out.append("; Path=", 7);
325+
out.append("; Path=");
357326
out.append(path_);
358327
}
359328
}
@@ -363,10 +332,10 @@ void cookie::append_flag_attributes(std::string& out) const {
363332
const bool effective_secure =
364333
secure_ || (same_site_ == same_site_mode::none);
365334
if (effective_secure) {
366-
out.append("; Secure", 8);
335+
out.append("; Secure");
367336
}
368337
if (http_only_) {
369-
out.append("; HttpOnly", 10);
338+
out.append("; HttpOnly");
370339
}
371340
}
372341

0 commit comments

Comments
 (0)