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

content length not available in header function, again #13752

Closed
piru opened this issue May 22, 2024 · 4 comments
Closed

content length not available in header function, again #13752

piru opened this issue May 22, 2024 · 4 comments

Comments

@piru
Copy link

piru commented May 22, 2024

I did this

Use CURLINFO_CONTENT_LENGTH_DOWNLOAD_T in CURLOPT_HEADERFUNCTION function when the last header is processed (empty newline is received). Rather than getting the content length -1 is returned. The call working in this specific scenario is undocumented behaviour that some (broken) code relies on. This issue was tracked earlier as #7804 and has surfaced again couple of versions ago (not sure when exactly).

I expected the following

Maybe this behaviour should be maintained since for example WebKit curl backend (*) is broken now. However, this is undocumented behaviour after all, so not fixing it is perfectly fine as well. WebKit and other code depending on this should be fixed either way.

*) https://github.com/WebKit/WebKit/blob/57affb62969db66ef07975567f4876d62d86da55/Source/WebCore/platform/network/curl/CurlRequest.cpp#L320

curl/libcurl version

curl 8.8.0

operating system

Linux hostname 6.8.9-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.8.9-1 (2024-05-15) x86_64 GNU/Linux

@icing
Copy link
Contributor

icing commented May 22, 2024

it's http.c:3630 and following where the \r\n is written to the client before the analysis and the info things are set.
To get what you want, the "Curl_client_write()" needs to be done after "http_on_response()" is called.

@piru
Copy link
Author

piru commented May 22, 2024

Ok that works at least in initial tests. I also had to defer Curl_dyn_reset(&data->state.headerb); to occur only after the Curl_client_write() call.

@piru
Copy link
Author

piru commented May 23, 2024

Just moving the Curl_dyn_reset later is wrong though, at least when considering the comment at

curl/lib/http.c

Line 3663 in 1c4813c

/* Caveat: we clear anything in the header brigade, because a

I tried creating a dynbuf copy of the hd, hdlen data and then doing Curl_dyn_reset as before, and using the copy for the Curl_client_write. It works (tests pass), but if there is recursion the order of header callbacks changes. I don't feel confident if this would be correct.

icing added a commit to icing/curl that referenced this issue May 23, 2024
- HEADERFUNCTIONS might inspect response properties like
  CURLINFO_CONTENT_LENGTH_DOWNLOAD_T on seeing the last
  header line. If the line is being written before this
  is initialized, values are not available.
- write the last header line late when analyzing a HTTP
  response so that all information is available at the
  time of the writing.
- add test1485 to verify that CURLINFO_CONTENT_LENGTH_DOWNLOAD_T
  works on seeing the last header.
- refs curl#13752
@icing
Copy link
Contributor

icing commented May 23, 2024

My proposal on how to solve this - test case included: #13757.

@bagder bagder closed this as completed in 17af2bc May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants