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

libp2phttp: workaround for ResponseWriter's CloseNotifier #2821

Merged
merged 2 commits into from
Jun 10, 2024

Conversation

MarcoPolo
Copy link
Contributor

ResponseWriter implements a deprecated CloseNotifier interface. The implementation considers any error during a read an indication that the whole connection is closed. One of these errors is EOF.

When we do HTTP over streams, it's normal for an EOF to happen after reading the request (in fact, it's recommended in the spec).

The workaround here is to ignore EOF errors on read when it's a stream we accept a stream (Listener side). This should be fine since we know how to read a full request (doesn't rely on EOF) and if the stream is closed writing will fail

@MarcoPolo MarcoPolo requested a review from sukunrt June 5, 2024 00:17
@MarcoPolo MarcoPolo merged commit 3338f24 into master Jun 10, 2024
11 checks passed
sukunrt pushed a commit that referenced this pull request Jun 12, 2024
* libp2phttp: workaround for CloseNotifier

* Add lintignore
sukunrt pushed a commit that referenced this pull request Jun 12, 2024
* libp2phttp: workaround for CloseNotifier

* Add lintignore
MarcoPolo added a commit that referenced this pull request Jun 12, 2024
* identify: Don't filter addr if remote is neither public nor private (#2820)

Updates the filterAddrs logic to no-op if the address is neither public nor private.

This fixes an issue in mocknet that assigns each node an address in the IPv6 discard prefix space. That doesn't interact well with this logic in identify.

The issue mocknet hits is that it filters out all received listen addresses and then doesn't remember any address for the peer.

* identify: fix bug in observed address handling (#2825)

* identify: add test for observed address handling (#2828)

This modifies TestObservedAddrManager to verify the fix in #2825

* libp2phttp: workaround for ResponseWriter's CloseNotifier (#2821)

* libp2phttp: workaround for CloseNotifier

* Add lintignore

* circuitv2: improve voucher validation (#2826)

* webrtc: fix ufrag prefix for dialing (#2832)

* webrtc: add a test for establishing many connections (#2801)

Update pion/ice to include the fix for out of order 
ConnectionState update callbacks

* release v0.35.1

---------

Co-authored-by: Marco Munizaga <git@marcopolo.io>
Co-authored-by: Ivan Shvedunov <ivan4th@users.noreply.github.com>
MarcoPolo added a commit that referenced this pull request Jun 13, 2024
* libp2phttp: workaround for CloseNotifier

* Add lintignore
MarcoPolo added a commit that referenced this pull request Jun 13, 2024
* libp2phttp: workaround for CloseNotifier

* Add lintignore
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.

None yet

2 participants