Skip to content

Fixed missing space in cookie header when using multiple -b flags - #20184

Closed
pojomi wants to merge 17 commits into
curl:masterfrom
pojomi:fix-cookie-spacing
Closed

Fixed missing space in cookie header when using multiple -b flags#20184
pojomi wants to merge 17 commits into
curl:masterfrom
pojomi:fix-cookie-spacing

Conversation

@pojomi

@pojomi pojomi commented Jan 4, 2026

Copy link
Copy Markdown
Contributor

Summary

When using multiple -b flags, the Cookie header was missing a space after the semicolon separator.

Bug behavior

curl -b 'a=b' -b 'c=d' https://google.com/

Produced: Cookie: a=b;c=d (missing space)

Expected behavior

Should produce: Cookie: a=b; c=d (space after semicolon)

Fix

Changed the format string in src/config2setopts.c line 550 from ";%s" to "; %s" to include the space separator as per HTTP cookie header specifications.

Testing

Verified with:

./src/curl -b 'a=b' -b 'c=d' https://google.com/ -v

Now correctly outputs: Cookie: a=b; c=d

Fixes #20183 #20182

@github-actions github-actions Bot added the tests label Jan 4, 2026
@pojomi
pojomi marked this pull request as draft January 4, 2026 21:24
@testclutch

Copy link
Copy Markdown

Analysis of PR #20184 at b0b09591:

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

Test 2500 failed, but it has been 32.1% flaky lately, so it's probably NOT a fault of the PR. Note that this test has failed in 4 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (3, to be exact) so it may be that this failure is rather a systemic issue with this job and not with this specific PR.

Generated by Testclutch

@bagder

bagder commented Jan 4, 2026

Copy link
Copy Markdown
Member

Isn't it enough to just:

@@ -545,11 +545,12 @@ static CURLcode cookie_setopts(struct OperationConfig *config, CURL *curl)
     curlx_dyn_init(&cookies, MAX_COOKIE_LINE);
     for(cl = config->cookies; cl; cl = cl->next) {
       if(cl == config->cookies)
         result = curlx_dyn_add(&cookies, cl->data);
       else
-        result = curlx_dyn_addf(&cookies, ";%s", cl->data);
+        result = curlx_dyn_addf(&cookies, ";%s%s",
+                                ISBLANK(cl->data[0]) ? "": " ", cl->data);
 
       if(result) {
         warnf("skipped provided cookie, the cookie header "
               "would go over %u bytes", MAX_COOKIE_LINE);
         return result;

@pojomi
pojomi marked this pull request as ready for review January 4, 2026 22:29
@pojomi

pojomi commented Jan 4, 2026

Copy link
Copy Markdown
Contributor Author

Isn't it enough to just:

@@ -545,11 +545,12 @@ static CURLcode cookie_setopts(struct OperationConfig *config, CURL *curl)
     curlx_dyn_init(&cookies, MAX_COOKIE_LINE);
     for(cl = config->cookies; cl; cl = cl->next) {
       if(cl == config->cookies)
         result = curlx_dyn_add(&cookies, cl->data);
       else
-        result = curlx_dyn_addf(&cookies, ";%s", cl->data);
+        result = curlx_dyn_addf(&cookies, ";%s%s",
+                                ISBLANK(cl->data[0]) ? "": " ", cl->data);
 
       if(result) {
         warnf("skipped provided cookie, the cookie header "
               "would go over %u bytes", MAX_COOKIE_LINE);
         return result;

@bagder
Test 6 uses -b "name=contents;name2=content2" which has semicolons within a single flag. The helper function handles those internal semicolons.

@bagder

bagder commented Jan 4, 2026

Copy link
Copy Markdown
Member

Test 6 uses -b "name=contents;name2=content2" which has semicolons within a single flag. The helper function handles those internal semicolons.

I think we should let the user's data be passed in as-is.

@pojomi

pojomi commented Jan 5, 2026

Copy link
Copy Markdown
Contributor Author

Test 6 uses -b "name=contents;name2=content2" which has semicolons within a single flag. The helper function handles those internal semicolons.

I think we should let the user's data be passed in as-is.

You're right. Simplified to only resolve the issue reported.

@pojomi
pojomi marked this pull request as draft January 5, 2026 00:50
@pojomi
pojomi marked this pull request as ready for review January 5, 2026 01:24
Comment thread src/config2setopts.c
Comment thread src/config2setopts.c Outdated
Comment thread src/config2setopts.c Outdated
Comment thread src/config2setopts.c Outdated
Comment thread src/config2setopts.c Outdated
Comment thread src/config2setopts.c Outdated
Comment thread src/config2setopts.c Outdated
Comment thread src/config2setopts.c Outdated
@bagder bagder closed this in dc739fe Jan 5, 2026
@bagder

bagder commented Jan 5, 2026

Copy link
Copy Markdown
Member

Thanks!

@pojomi

pojomi commented Jan 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks!

Thank you for approving my first PR! More to come.

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.

curl Joins Multiple Cookie Strings without Spaces

4 participants