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

refine: return error when response writer doesn't plement hijacker #4026

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xieyuschen
Copy link

@xieyuschen xieyuschen commented Aug 7, 2024

The change tries to align with the H2C behavior, which returns an error when the response writer doesn't implement the hijacker api. https://github.com/golang/net/blob/master/http2/h2c/h2c.go#L137

The panic could be demonstrated as the gist reveals: https://gist.github.com/xieyuschen/ff525642e576de14729b9b9214c656f5

I raised this PR because during our H2C integration with gin, we found this potential panic which is better to be fixed from the framework side. It's an improvement specifically to the HTTP/2 case. It's not a breaking change and could help integrate gin with H2C.

Regards

@xieyuschen
Copy link
Author

Hi @appleboy @manucorporat how do you think about this change?

@xieyuschen
Copy link
Author

@thinkerou

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

Successfully merging this pull request may close these issues.

1 participant