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

Reset connection after HEAD? #258

Closed
ronag opened this issue Jul 14, 2020 · 10 comments · Fixed by #262
Closed

Reset connection after HEAD? #258

ronag opened this issue Jul 14, 2020 · 10 comments · Fixed by #262
Labels
question [Use a Discussion instead]

Comments

@ronag
Copy link
Member

ronag commented Jul 14, 2020

Continuing from nodejs/node#34350.

Some servers send a body with HEAD requests which is kind of un-parsable. There is however one way to be resilient against it. Always reset the connection after receiving the message headers for a HEAD request.

This would make things significantly slower though but much more resilient. How important are HEAD requests?

Should we just ignore this unfortunate case and hope that the target server behaves properly?

Opt-in/Opt-out option?

@ronag
Copy link
Member Author

ronag commented Jul 14, 2020

@szmarczak @delvedor

@ronag ronag added the question [Use a Discussion instead] label Jul 14, 2020
@szmarczak
Copy link
Member

Can't we just throw on transfer-encoding: chunked?

@szmarczak
Copy link
Member

If there's any data that we don't expect, we can also reset the connection at that point.

@ronag
Copy link
Member Author

ronag commented Jul 14, 2020

Can't we just throw on transfer-encoding: chunked?

That's a valid header for a HEAD response?

If there's any data that we don't expect, we can also reset the connection at that point.

Hard to determine. What if the extra data (i.e. HEAD body) contains data that looks like a proper response?

@szmarczak
Copy link
Member

szmarczak commented Jul 14, 2020

That's a valid header for a HEAD response?

Ah, you're right.

What if the extra data (i.e. HEAD body) contains data that looks like a proper response?

Then resetting the connection would make sense, but indeed, it would be a significant performance drop. But there are lots of edge cases like that so I wouldn't go for this.

IMO, if it looks like a proper response, it should be parsed, and if the path doesn't match with the pipelined request (or there isn't any) then throw reconnect.

@szmarczak
Copy link
Member

But there are lots of edge cases like that so I wouldn't go for this.

E.g. GET receives content-length: 0 but there is actually a body...

@ronag
Copy link
Member Author

ronag commented Jul 14, 2020

Yea, I guess it's not really solvable.

@ronag
Copy link
Member Author

ronag commented Jul 14, 2020

E.g. GET receives content-length: 0 but there is actually a body...

We could reset connection after content-length: 0 as well 😄

@ronag
Copy link
Member Author

ronag commented Jul 19, 2020

So it turns out that Node actually does reset connection after HEAD. See, nodejs/node#12396.

I think it might make sense for us to do so as well.

Thoughts? @mcollina @szmarczak

@mcollina
Copy link
Member

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question [Use a Discussion instead]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants