Skip to content

cookie: compare path case sensitively#21616

Closed
bagder wants to merge 2 commits into
masterfrom
bagder/cookie-path
Closed

cookie: compare path case sensitively#21616
bagder wants to merge 2 commits into
masterfrom
bagder/cookie-path

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented May 14, 2026

Verify with test 1645

Reported-by: Joshua Rogers

Verify with test 1645

Reported-by: Joshua Rogers
@bagder bagder added the cookies label May 14, 2026
@github-actions github-actions Bot added the tests label May 14, 2026
@bagder bagder marked this pull request as ready for review May 14, 2026 22:08
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates libcurl’s cookie handling so cookie Path comparisons are performed case-sensitively, and adds a regression test to verify that cookies with the same name/domain but different-cased paths are treated as distinct entries in the cookie jar.

Changes:

  • Switch cookie path comparisons in replace_existing() from case-insensitive helpers to strncmp/strcmp (case-sensitive).
  • Add new test test1645 to verify cookie-jar output preserves two cookies whose paths differ only by case.
  • Register test1645 in the tests data Makefile.

Reviewed changes

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

File Description
tests/data/test1645 New regression test validating distinct cookie-jar entries for same-name cookies on paths differing only by case.
tests/data/Makefile.am Adds test1645 to the test list so it runs in the suite.
lib/cookie.c Makes path comparison logic case-sensitive when deciding overlay/replacement behavior for existing cookies.

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

Comment thread lib/cookie.c
@testclutch
Copy link
Copy Markdown

Analysis of PR #21616 at 9986afb3:

Test 1501 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 12 different CI jobs (the link just goes to one of them).

Generated by Testclutch

@bagder bagder closed this in a15cfeb May 15, 2026
@bagder bagder deleted the bagder/cookie-path branch May 15, 2026 11:39
outcast36 pushed a commit to greearb/curl that referenced this pull request Jun 3, 2026
Verify with test 1645

Reported-by: Joshua Rogers
Closes curl#21616
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