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

header api: add curl_easy_header and curl_easy_nextheader #8593

Closed
wants to merge 9 commits into from

Conversation

bagder
Copy link
Member

@bagder bagder commented Mar 14, 2022

Early docs: https://github.com/curl/curl/wiki/get-headers-v2

  • code for the two functions
  • man pages for the functions
  • example showing the API functions in use
  • add test case for curl_easy_header
  • figure out how to deal with CONNECT response headers
  • figure out how to deal with 1xx response headers
  • add test case for curl_easy_header with CONNECT
  • documented as EXPERIMENTAL
  • add test case for curl_easy_header with 1xx
  • add test case for curl_easy_header with trailers
  • add test case for curl_easy_header in a redirect case
  • test case for curl_easy_nextheader
  • make it build fine when HTTP support is disabled
  • use a single malloc per stored header
  • make the function calls able to access headers from multiple responses from the transfer
  • allow curl -w to output header contents with %header{name}
  • supports -w %{header_json} - ouputs all headers as JSON
  • make -w %{header_json} handle duplicate headers
  • add test case for -w %header{name}
  • add test case for -w %{header_json}
  • support/handle psuedo headers
  • skip making a test for psuedo headers (due to lacking test infra)
  • make the API (and -w options) build opt-in while EXPERIMENTAL
  • make the tests run only if the headers API is enabled in the build
  • make sure the docs appear nicely on the website (pending PR)

Remove the wiki page (to keep the docs in a single place) after initial merge

@jay
Copy link
Member

@jay jay commented Mar 14, 2022

does this cover 100 status responses, for example

HTTP/1.1 101 Switching Protocols
Upgrade: websocket
Connection: Upgrade

@bagder

This comment has been hidden.

@bagder

This comment has been hidden.

@bagder

This comment was marked as outdated.

@bagder bagder force-pushed the bagder/header-api branch 3 times, most recently from 1f8b020 to ebdd863 Compare Mar 15, 2022
@bagder

This comment has been hidden.

bagder added a commit that referenced this issue Mar 17, 2022
Add test 1940 to 1946 to verify.

Closes #8593
lib/headers.c Show resolved Hide resolved
bagder added a commit that referenced this issue Mar 18, 2022
Add test 1940 to 1946 to verify.

Closes #8593
bagder added a commit that referenced this issue Mar 19, 2022
Add test 1940 to 1946 to verify.

Closes #8593
bagder added a commit that referenced this issue Mar 19, 2022
Add test 1940 to 1946 to verify.

Closes #8593
@bagder bagder marked this pull request as ready for review Mar 19, 2022
@bagder
Copy link
Member Author

@bagder bagder commented Mar 20, 2022

I'm ready to merge this within the next few days. Any comments to take into account before that happens?

@bagder bagder deleted the bagder/header-api branch Mar 22, 2022
@nanonyme
Copy link

@nanonyme nanonyme commented Apr 28, 2022

Hmm, was this supposed to hide the new experimental ABI by default if --enable-headers-api is not given? Because it doesn't seem it does.

@bagder
Copy link
Member Author

@bagder bagder commented Apr 28, 2022

Can you clarify what you're asking?

@nanonyme
Copy link

@nanonyme nanonyme commented Apr 28, 2022

This post clearly states that these new functions should not be exposed as they are experimental. Yet it looks based on our ABI checks that they are indeed exposed by default even without the flag to expose them https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/jobs/2386983527 Is this intended behaviour or should I file a bug?

@bagder
Copy link
Member Author

@bagder bagder commented Apr 28, 2022

The functions are present even when disabled, but they don't do anything. They have no functionality.

@nanonyme
Copy link

@nanonyme nanonyme commented Apr 28, 2022

I see. But just exposing them means people may call them. As it was said, the ABI is subject to change up to function signatures so isn't it completely unsafe to keep them there even as no-op functions?

@bagder
Copy link
Member Author

@bagder bagder commented Apr 28, 2022

Why would someone call them if they don't do anything? And yes, providing experimental features risks that users don't read our warnings and go ahead and use them anyway. I don't see an easy way to prevent that.

@nanonyme
Copy link

@nanonyme nanonyme commented Apr 28, 2022

Couldn't you just use USE_HEADERS_API macro to hide them as private symbols?

@dfandrich
Copy link
Collaborator

@dfandrich dfandrich commented Apr 28, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants