Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bad argument error if there is a white space at first header. #11605

Closed
junsik opened this issue Aug 7, 2023 · 6 comments
Closed

bad argument error if there is a white space at first header. #11605

junsik opened this issue Aug 7, 2023 · 6 comments
Assignees
@bagder
Labels
HTTP

Comments

@junsik
Copy link

junsik commented Aug 7, 2023 •
edited

I did this

curl 8.2.0 tells bad argument error if there is a white space at first header.

* processing: http://www.w4c.go.kr/intro/introFcltMainSttus.do
*   Trying 27.101.219.129:80...
* Connected to www.w4c.go.kr (27.101.219.129) port 80
> GET /intro/introFcltMainSttus.do HTTP/1.1
Host: www.w4c.go.kr
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.111 Safari/537.36
Accept: */*
Accept-Encoding: deflate, gzip

< HTTP/1.1 200 OK
<  X-Content-Type-Options: nosniff
* Closing connection

XHR Error: (43) A libcurl function was given a bad argument

this might be related with folded header patch: b7baa78

A header name cannot have any whitespace at RFC.

      message-header = field-name ":" [ field-value ]
       field-name     = token
       field-value    = *( field-content | LWS )
       field-content  = <the OCTETs making up the field-value
                        and consisting of either *TEXT or combinations
                        of token, separators, and quoted-string>


  token          = 1*tchar

  tchar          = "!" / "#" / "$" / "%" / "&" / "'" / "*"
                 / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
                 / DIGIT / ALPHA
                 ; any VCHAR, except delimiters

But I am not sure strict manger is right or not.
For fixing it, following code could be considered:

if((header[0] == ' ') || (header[0] == '\t')) {
    if(data->state.prevhead)
      /* line folding, append value to the previous header's value */
      return unfold_value(data, header, hlen);
  }

I expected the following

curl 7.68.0

$ curl http://www.w4c.go.kr -v
*   Trying 27.101.219.129:80...
* TCP_NODELAY set
* Connected to www.w4c.go.kr (27.101.219.129) port 80 (#0)
> GET / HTTP/1.1
> Host: www.w4c.go.kr
> User-Agent: curl/7.68.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 302 Found
<  X-Content-Type-Options: nosniff
< X-XSS-Protection: 1; mode=block
< Date: Mon, 07 Aug 2023 06:10:26 GMT
< Connection: close
< Location: /server_busy
< Content-Type: text/html
< Content-Length: 140

curl/libcurl version

curl 8.2.0

operating system

No response
@bagder bagder added the HTTP label Aug 7, 2023
@bagder bagder self-assigned this Aug 7, 2023
@bagder
Copy link
Member

bagder commented Aug 7, 2023

I don't think curl is wrong here (but the error message is confusing).

but since the popular browsers seem to silently deal with this, I suppose curl should as well.

@junsik
Copy link
Author

junsik commented Aug 7, 2023

thank you for quick reply.

bagder added a commit that referenced this issue Aug 7, 2023
@bagder
headers: silently accept leading whitespace on first response header
74233b2 
This is a bad header fold but since the popular browsers accept this
violation, does curl now.

Add test 1473 to verify.

Reported-by: junsik on github
Fixes #11605
@bagder bagder mentioned this issue Aug 7, 2023
Closed
bagder added a commit that referenced this issue Aug 7, 2023
@bagder
headers: accept leading whitespaces on first response header
c4d1806 
This is a bad header fold but since the popular browsers accept this
violation, so does curl now.

Add test 1473 to verify and adjust test 2306.

Reported-by: junsik on github
Fixes #11605
Closes #11607
@junsik
Copy link
Author

junsik commented Aug 7, 2023

thank you for quick patch.

@junsik junsik closed this as completed Aug 7, 2023
bagder added a commit that referenced this issue Aug 7, 2023
@bagder
headers: accept leading whitespaces on first response header
95ca3c5 
This is a bad header fold but since the popular browsers accept this
violation, so does curl now. Unless built with hyper.

Add test 1473 to verify and adjust test 2306.

Reported-by: junsik on github
Fixes #11605
Closes #11607
@junsik
Copy link
Author

junsik commented Aug 8, 2023

At persistent connection case, this patch makes a segfault.
Previous header data might be effected.

Below is second curl_easy_perform()'s back trace:

#0  __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:120
#1  0x00005643c7b6989b in unfold_value (vlen=33, value=0x5643ca61a7f0 " X-Content-Type-Options: nosniff\r\n", 
    data=0x5643ca5104e0) at headers.c:231
#2  Curl_headers_push (data=data@entry=0x5643ca5104e0, 
    header=header@entry=0x5643ca61a7f0 " X-Content-Type-Options: nosniff\r\n", type=<optimized out>) at headers.c:301
#3  0x00005643c7b4df9b in chop_write (olen=34, optr=0x5643ca61a7f0 " X-Content-Type-Options: nosniff\r\n", 
    optr@entry=0x5643ca5104e0 "\255\333\336\300", type=2, type@entry=0, data=0x5643ca5104e0, data@entry=0x4000)
    at sendf.c:610
#4  Curl_client_write (data=data@entry=0x5643ca5104e0, type=type@entry=2, 
    ptr=ptr@entry=0x5643ca61a7f0 " X-Content-Type-Options: nosniff\r\n", len=34) at sendf.c:664
#5  0x00005643c7b6f6e9 in Curl_http_readwrite_headers (data=data@entry=0x5643ca5104e0, 
    conn=conn@entry=0x5643ca51cb80, nread=nread@entry=0x7fff800bbb40, stop_reading=stop_reading@entry=0x7fff800bbb3c)
    at http.c:4449
#6  0x00005643c7b570d8 in readwrite_data (comeback=0x7fff800bbbb7, done=0x7fff800bbbb5, didwhat=<synthetic pointer>, 
    k=0x5643ca5105b0, conn=0x5643ca51cb80, data=0x5643ca5104e0) at transfer.c:633
#7  Curl_readwrite (conn=0x5643ca51cb80, data=data@entry=0x5643ca5104e0, done=done@entry=0x7fff800bbbb5, 
    comeback=comeback@entry=0x7fff800bbbb7) at transfer.c:1219
#8  0x00005643c7b49ab4 in multi_runsingle (multi=multi@entry=0x5643ca3af300, nowp=nowp@entry=0x7fff800bbc30, 
    data=data@entry=0x5643ca5104e0) at multi.c:2404
#9  0x00005643c7b4af2e in curl_multi_perform (multi=multi@entry=0x5643ca3af300, 
    running_handles=running_handles@entry=0x7fff800bbd38) at multi.c:2682
#10 0x00005643c7b3ff93 in easy_transfer (multi=<optimized out>) at easy.c:663
#11 easy_perform (events=false, data=0x5643ca5104e0) at easy.c:753
#12 curl_easy_perform (data=0x5643ca5104e0) at easy.c:772

@junsik junsik reopened this Aug 8, 2023
@bagder
Copy link
Member

bagder commented Aug 8, 2023 •
edited

  1. This is not the same bug.
  2. Can you please tell us how you reproduce this? We have test 2306 that does a leading space on a re-used connection but clearly that does not show this problem.

@bagder bagder mentioned this issue Aug 8, 2023
Closed
@bagder
Copy link
Member

bagder commented Aug 8, 2023

See #11620 for the new problem

@bagder bagder closed this as completed Aug 8, 2023
ptitSeb pushed a commit to wasix-org/curl that referenced this issue Sep 25, 2023
@bagder @ptitSeb
headers: accept leading whitespaces on first response header
5f4d12f 
This is a bad header fold but since the popular browsers accept this
violation, so does curl now. Unless built with hyper.

Add test 1473 to verify and adjust test 2306.

Reported-by: junsik on github
Fixes curl#11605
Closes curl#11607
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bagder bagder

Labels
HTTP
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

headers: accept leading whitespace on first response header curl/curl
2 participants
@bagder @junsik