Skip to content

cookie: accept slightly longer lines in cookie jars - #22964

Closed
bagder wants to merge 1 commit into
masterfrom
bagder/cookie-load
Closed

bagder wants to merge 1 commit into
masterfrom
bagder/cookie-load

Conversation

@bagder

@bagder bagder commented Sep 16, 2026

Copy link
Copy Markdown
Member

No description provided.

@bagder bagder added the cookies label Sep 16, 2026
@github-actions github-actions Bot added the HTTP label Sep 16, 2026
@bagder
bagder marked this pull request as ready for review September 16, 2026 09:12
@testclutch

Copy link
Copy Markdown

Analysis of PR #22964 at a7cf5198:

Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_09_dnsd_a_delay[DoH] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_10_dnsd_aaaa_delay[DoH] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_13_dnsd_nxdomain_cached[DoH] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Generated by Testclutch

Copilot AI 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.

🟡 Changes recommended

The line-limit divergence needs resolution and boundary regression coverage, with a minor comment correction also requested.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adjusts cookie-jar loading to accept slightly longer lines without aborting file reads.

Changes:

  • Increases the dynamic cookie-jar buffer by 48 bytes.
  • Adds rationale for the increased limit.
File summaries
File Changes and findings
lib/cookie.c Expands the cookie-jar line buffer. Moderate (2 votes): document or enforce the differing Netscape line limit. Nit (3 votes): add boundary regression coverage. Nit (1 vote): correct the comment wording.
Review details

Suppressed comments (1)

lib/cookie.c:1185

  • The comment should say “slightly longer lines” (or “a few longer lines”); “a little longer lines” is grammatically incorrect.
    /* accept a little longer lines so that a full cookie fits and does not
       abort the file read */
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/cookie.c
Comment thread lib/cookie.c
@bagder bagder closed this in 445aa87 Sep 16, 2026
@bagder
bagder deleted the bagder/cookie-load branch September 16, 2026 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants