-
-
Notifications
You must be signed in to change notification settings - Fork 694
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
Ensure HTTP connections are closed in case of data-less TCP pings #1244
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -223,6 +223,11 @@ def handle_events(self): | |||||||||
self.cycle.more_body = False | ||||||||||
self.cycle.message_event.set() | ||||||||||
|
||||||||||
elif event_type is h11.ConnectionClosed: | ||||||||||
if not self.transport.is_closing(): | ||||||||||
self.transport.close() | ||||||||||
break | ||||||||||
Comment on lines
+226
to
+229
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
||||||||||
def handle_upgrade(self, event): | ||||||||||
upgrade_value = None | ||||||||||
for name, value in self.headers: | ||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -137,6 +137,9 @@ def data_received(self, data): | |||||||
except httptools.HttpParserUpgrade: | ||||||||
self.handle_upgrade() | ||||||||
|
||||||||
if data == b"" and not self.transport.is_closing(): | ||||||||
self.transport.close() | ||||||||
|
||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems like this pass tests and fix the issue if I'm correct |
||||||||
def handle_upgrade(self): | ||||||||
upgrade_value = None | ||||||||
for name, value in self.headers: | ||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we should be moving that
if data:
check toH11Protocol
instead, then.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the thing is that this
b""
case is already handled by h11 in it'sreceive_data
we use in our protocol'sdata_received
see https://gitlab.com/kalilinux/packages/python-h11/-/blob/kali/master/h11/_connection.py#L303-348
so I dont know