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

Remove duplicate Content-Length headers that are rejected by Puma 5.6.4. #536

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nialbima
Copy link

@nialbima nialbima commented Apr 5, 2022

Puma's 5.6.4 update prevents Content-Length headers of the form "125, 125" from being accepted by the server. Faye seems to be adding them in a number of locations, and removing the code that supported adding additional Content-Length headers to requests made it possible to continue to use our implementation of ServeryProxy to run integration tests against a simulated Faye backend running in Puma. Because rack_adapter_spec explicitly checks for the presence of Content-Length headers, this seems to resolve the bug.

@nialbima nialbima force-pushed the remove-duplicate-content-types branch from 270d1b6 to 9587fb4 Compare April 5, 2022 17:12
@nialbima nialbima closed this Apr 5, 2022
@nialbima nialbima reopened this Apr 5, 2022
@jcoglan
Copy link
Collaborator

jcoglan commented Apr 12, 2022

Could you explain further how you're observing headers of the form 125, 125? When I run the example server with Puma 5.6.4, it returns a well-formed content-length header:

$ curl -si http://localhost:7000/bayeux -H 'content-type: application/json' -d '{ "channel": "/meta/handshake", "supportedConnectionTypes": ["long-polling"], "version": "1.0" }'
HTTP/1.1 200 OK
Content-Type: application/json; charset=utf-8
Cache-Control: no-cache, no-store
X-Content-Type-Options: nosniff
Content-Length: 300

[{"channel":"/meta/handshake","successful":true,"version":"1.0","supportedConnectionTypes":["long-polling","cross-origin-long-polling","callback-polling","websocket","eventsource","in-process"],"clientId":"kia6okj2m84lhodxy7fdm2nk7deo2df","advice":{"reconnect":"retry","interval":0,"timeout":25000}}]

None of the points in the code where you've introduced changes should be producing headers like 125, 125, because they all stringify a single number that we got from String#bytesize.

@jcoglan
Copy link
Collaborator

jcoglan commented Apr 12, 2022

On the commit you're asking to pull from, the server doesn't return a content-length header at all, which is incorrect:

$ curl -si http://localhost:7000/bayeux -H 'content-type: application/json' -d '{ "channel": "/meta/handshake", "supportedConnectionTypes": ["long-polling"], "version": "1.0" }'
HTTP/1.1 200 OK
Content-Type: application/json; charset=utf-8
Cache-Control: no-cache, no-store
X-Content-Type-Options: nosniff

[{"channel":"/meta/handshake","successful":true,"version":"1.0","supportedConnectionTypes":["long-polling","cross-origin-long-polling","callback-polling","websocket","eventsource","in-process"],"clientId":"ncpmyfdtv7wz9gc7zm9yw2gnl55ar7o","advice":{"reconnect":"retry","interval":0,"timeout":25000}}]

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