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.

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

header_json emits incorrect JSON with multi-value headers #10704

Closed
inca opened this issue Mar 7, 2023 · 0 comments
Closed

header_json emits incorrect JSON with multi-value headers #10704

inca opened this issue Mar 7, 2023 · 0 comments
Assignees
@bagder
Labels
cmdline tool

Comments

@inca
Copy link

inca commented Mar 7, 2023 •
edited

I did this

There is a backend that returns several multi-value headers in overlapping order:

< HTTP/2 200 
< server: nginx
< date: Tue, 07 Mar 2023 15:14:41 GMT
< content-type: application/json
< content-length: 12265
< vary: Accept-Encoding
< access-control-allow-origin: *
< vary: Accept-Encoding
< referrer-policy: strict-origin-when-cross-origin
< access-control-allow-methods: GET, POST, PUT, DELETE, OPTIONS
< access-control-max-age: 1728000
< access-control-allow-headers: Authorization, Content-Type, AuthorizationOauth, X-EARLY-ACCESS
< access-control-expose-headers: 
< vary: Accept
< etag: W/"2678f9ab2ba550d164e7cc014aefd31e"
< cache-control: max-age=0, private, must-revalidate
< x-request-id: 375b343b3d2ecf9b442c0daf00fc4a9a
< strict-transport-security: max-age=31536000; includeSubDomains
< x-content-type-options: nosniff
< x-xss-protection: 1; mode=block
< referrer-policy: strict-origin-when-cross-origin
< feature-policy: accelerometer 'none'; camera 'none'; geolocation 'none'; gyroscope 'none'; magnetometer 'none'; microphone 'none'; payment 'none'; usb 'none'

You can see the vary and referrer-policy occurring multiple times. With -w %{header_json} option the JSON produced becomes incorrect:

{
	"server": ["nginx"],
	"date": ["Tue, 07 Mar 2023 11:31:47 GMT"],
	"content-type": ["application/json"],
	"content-length": ["12265"],
	"vary": ["Accept-Encoding", "Accept-Encoding", "Accept"],
	"etag": ["W/\"2678f9ab2ba550d164e7cc014aefd31e\""],
	"cache-control": ["max-age=0, private, must-revalidate"],
	"x-request-id": ["ad2176d584f1c5b1a56b564af1969efa"],
	"strict-transport-security": ["max-age=31536000; includeSubDomains"],
	"x-content-type-options": ["nosniff"],
	"x-xss-protection": ["1; mode=block"],
	"referrer-policy": ],
	"feature-policy": ["accelerometer 'none'; camera 'none'; geolocation 'none'; gyroscope 'none'; magnetometer 'none'; microphone 'none'; payment 'none'; usb 'none'"]
}

Notice the "referrer-policy": ],. Also notice that headers between the first occurrence of the first multi-value header (vary) are also missing.

Here's the cURL command to reproduce the issue (note: the token is taken from the official docs, so feel free to use it).

curl https://api.pagerduty.com/oncalls -H "Accept: application/vnd.pagerduty+json;version=2" -H "Content-Type: application/json" -H "Authorization: Token token=y_NbAkKc66ryYTWUXYEu" -w %{header_json}

I managed to track it down to headerJSON right here:

curl/src/tool_writeout_json.c

Lines 131 to 132 in a26418c

if(curl_easy_header(per->curl, name, i, CURLH_HEADER,
-1, &header))
— I suspect that prev pointer gets overwritten by that last call to curl_easy_header before breaking from the loop; therefore the next iteration of the while starts from etag (which is the next header after the last occurrence of vary) and therefore misses the first occurrence of another multi-value header referrer-policy (along with all other headers in between the first and the last occurrence of vary). By the time the outer loop reaches referrer-policy it actually sees the second occurrence (with index 1) and that causes the method to only emit a closing bracket because the branch before it is never entered.

I expected the following

JSON should be valid. All headers should be present.

curl/libcurl version

7.88.1

operating system

macOS 12.5.1

/cc @christellevs
@bagder bagder self-assigned this Mar 7, 2023
bagder added a commit that referenced this issue Mar 7, 2023
@bagder
test421: -w %{header_json} test with multiple same header names
ed9bc02 
To reproduce the issue in #10704
@bagder bagder mentioned this issue Mar 7, 2023
Closed
@akabanovs akabanovs mentioned this issue Mar 8, 2023
Merged
bagder added a commit that referenced this issue Mar 8, 2023
@bagder
headers: make curl_easy_header and nextheader return different buffers
b6afffe 
By letting curl_easy_header() and curl_easy_nextheader() store the
header data in their own struct storage when they return a pointer to
it, it makes it possible for applications to use them both in a loop.
Like the curl tool does.

Reported-by: Boris Okunskiy
Fixes #10704
Closes #10707
bagder added a commit that referenced this issue Mar 8, 2023
@bagder
tool_writeout_json. fix the output for duplicate header names
5782774 
Header entries with index != 0 are handled at the index 0 level so they
should then be skipped when iterated over.

Reported-by: Boris Okunskiy
Fixes #10704
Closes #10707
bagder added a commit that referenced this issue Mar 8, 2023
@bagder
test421: -w %{header_json} test with multiple same header names
5a01f77 
To reproduce the issue in #10704
@bagder bagder closed this as completed in 0561637 Mar 8, 2023
bagder added a commit that referenced this issue Mar 8, 2023
@bagder
tool_writeout_json. fix the output for duplicate header names
842be67 
Header entries with index != 0 are handled at the index 0 level so they
should then be skipped when iterated over.

Reported-by: Boris Okunskiy
Fixes #10704
Closes #10707
bagder added a commit that referenced this issue Mar 8, 2023
@bagder
test421: -w %{header_json} test with multiple same header names
27a7600 
To reproduce the issue in #10704
bch pushed a commit to bch/curl that referenced this issue Jul 19, 2023
@bagder @bch
headers: make curl_easy_header and nextheader return different buffers
d740785 
By letting curl_easy_header() and curl_easy_nextheader() store the
header data in their own struct storage when they return a pointer to
it, it makes it possible for applications to use them both in a loop.
Like the curl tool does.

Reported-by: Boris Okunskiy
Fixes curl#10704
Closes curl#10707
bch pushed a commit to bch/curl that referenced this issue Jul 19, 2023
@bagder @bch
tool_writeout_json. fix the output for duplicate header names
492da2f 
Header entries with index != 0 are handled at the index 0 level so they
should then be skipped when iterated over.

Reported-by: Boris Okunskiy
Fixes curl#10704
Closes curl#10707
bch pushed a commit to bch/curl that referenced this issue Jul 19, 2023
@bagder @bch
test421: -w %{header_json} test with multiple same header names
f98dfaf 
To reproduce the issue in curl#10704
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bagder bagder

Labels
cmdline tool
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

headers json output cleanup curl/curl
2 participants
@inca @bagder