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

fix(flash): Always send correct number of bytes when handling HEAD requests #17740

Merged
merged 2 commits into from
Feb 12, 2023
Merged

fix(flash): Always send correct number of bytes when handling HEAD requests #17740

merged 2 commits into from
Feb 12, 2023

Conversation

kamilogorek
Copy link
Contributor

@kamilogorek kamilogorek commented Feb 11, 2023

This was not caught in the previous test case, as the response body was smaller than the size of HEAD response.
This made nwritten < responseLen check in writeFixedResponse to fail, and not trigger op_flash_respond_async as a result.

When the response body is larger than the HEAD though, as in the updated test case (HEAD i 120 bytes, where our response is 300 bytes), it would think that we still have something to send, and effectively panic, as op_flash_respond already removed the request from the pool.

This change, makes the handleResponse function always calculate the number of bytes to transmit when HEAD request is encountered. Effectively ignoring Content-Length of the body, but still setting it correctly in the request header itself.

Fixes #17737

@kamilogorek kamilogorek changed the title Fix head response fix(flash): Always send correct number of bytes when handling HEAD requests Feb 11, 2023
Copy link
Collaborator

@aapoalas aapoalas left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@littledivy littledivy merged commit d4e5a29 into denoland:main Feb 12, 2023
@kamilogorek kamilogorek deleted the fix-head-response branch February 12, 2023 10:50
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.

Panic on Deno.server with a HEAD request
3 participants