Telegraph icon indicating copy to clipboard operation
Telegraph copied to clipboard

Fix "HEAD" Request Issue

Open ivanouanton opened this issue 2 years ago • 2 comments

Summary This pull request addresses the issue with the "HEAD" request not working by changing the http_parser_init function's parameter from HTTP_BOTH to HTTP_REQUEST. This change ensures that the "HEAD" request is correctly processed, resolving the previous problem.

Details In the current implementation, the http_parser_init function is invoked with the parameter HTTP_BOTH, which handles both requests and responses. However, this causes a specific issue with the "HEAD" request, preventing it from functioning correctly. By changing the parameter to HTTP_REQUEST, we explicitly instruct the parser to handle only HTTP requests.

This change has been tested and has been found to fix the "HEAD" request issue as expected.

Testing To verify the effectiveness of this fix, I ran comprehensive test cases covering various HTTP methods, including "HEAD" requests, using the updated http_parser_init with HTTP_REQUEST. All tests passed successfully, and the "HEAD" requests now return the expected responses.

Additional Observations While investigating the issue, I noticed a similar initialization in a forked framework http-parser, specifically in the test.c file of the HTTP_REQUEST fork.

Closing Remarks This contribution aims to improve the overall functionality of the repository by fixing the "HEAD" request issue. I have thoroughly tested the changes and ensured that they are in compliance with the coding standards of the project.

Looking forward to feedback and further discussion on this improvement. Thank you for reviewing this pull request.

ivanouanton avatar Aug 01 '23 11:08 ivanouanton

@yvbeek @Vernadsky could you please check this PR out? We need HEAD routes to be working properly.

rssole avatar Oct 19 '23 08:10 rssole

Please allow this fix, I spent hours trying to work out what the issue was until I found this fork. It works and it's simple. Thanks

Dalchrome avatar Jan 13 '24 19:01 Dalchrome

@ivanouanton Thank you for the PR.

I've had a deeper look into this issue. As http_parser is no longer maintained, I've modified the HTTPParserC dependency so that it now uses llhttp. This has also resolved the HEAD request issue that you reported here.

I've also added an example of a HEAD request to the demo app. Please try the latest version as soon as I have published it and let me know if that resolves your issues.

yvbeek avatar Mar 31 '24 20:03 yvbeek