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

[http2]: enhance error messages on Curl_dyn* upon receiving headers #16536

Closed

Conversation

starrify
Copy link
Contributor

@starrify starrify commented Mar 2, 2025

This is a partial fix of #16535. The error message format is borrowed from the existing code1.

Sample message before:
curl: (56) process_pending_input: nghttp2_session_mem_recv() returned -902:The user callback function failed

Sample message after:
curl: (56) Failed receiving HTTP2 header: 100(A value or data field grew larger than allowed)

Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I propose some minor changes.

@bagder bagder requested a review from icing March 2, 2025 23:24
@starrify starrify force-pushed the h2-max-http-header-error-handling-16535 branch from ed04b5d to ab23825 Compare March 2, 2025 23:35
@starrify
Copy link
Contributor Author

starrify commented Mar 2, 2025

Thanks a lot! I've updated the branch (to 5743423) to reflect the suggested changes.

This is a partial fix of curl#16535. The error message format is borrowed
from the existing code[1].

Sample message before:
    curl: (56) process_pending_input: nghttp2_session_mem_recv() returned -902:The user callback function failed

Sample message after:
    curl: (56) Error receiving HTTP2 header: 100(A value or data field grew larger than allowed)

[1]: https://github.com/curl/curl/blob/df672695e5992ad9b99819e9950de682e243cb48/lib/http2.c#L1999-L2000
@starrify starrify force-pushed the h2-max-http-header-error-handling-16535 branch from ab23825 to 5743423 Compare March 2, 2025 23:39
@bagder bagder closed this in f61b218 Mar 3, 2025
@bagder
Copy link
Member

bagder commented Mar 3, 2025

Thanks!

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.

3 participants