[http] Handle aborted http Requests #450
Conversation
| 11: { | ||
| in: "ToTAlly \\ Non REAdable HTTTTTTp pRotOcOlz", | ||
| err: "EOF", | ||
| headers: [] |
There was a problem hiding this comment.
This test doesn't demonstrate the error. When I run it without your other change, it passes.
There was a problem hiding this comment.
Mm you're right. We may work on a way to mock the request flow in the test runner. I'll try to find a way.
There was a problem hiding this comment.
Have you looked at these tests? https://github.com/golang/go/blob/master/src/net/http/requestwrite_test.go
They're quite nice. I think we need to add ResponseWriter.
There was a problem hiding this comment.
In this case it will not be revelant because we will not write the response. But yes it would be a good addition sure. Adding this in another PR?
zekth
changed the title
| }); | ||
| // in case of wrong first line HTTP header or | ||
| // aborted request we don't have to respond | ||
| if (req) { |
There was a problem hiding this comment.
I think we still run the risk of responding to the previous request.
This could be fixed by adding req = undefined to the start of the while loop above.
There was a problem hiding this comment.
TBH i don't know if this case could happen. As i haven't found a way to test it. I trust you on this.
|
I believe this issue was resolved by #546, can we close this PR now? |
|
Yes, I think this is fixed, but not yet released. |
|
Closing... thanks @zekth |
This also includes some related cleanups and fixes.
fix: #442
All the reward go to @MarkTiedemann . Great work on investigation.
Btw we may need to think about working on some e2e tests for the server, handling aborted connections etc. (dial does not provide this capability atm)