Don't set Content-Length for GET, HEAD, DELETE, or CONNECT requests without a BODY #684
Conversation
|
For context, node-fetch/node-fetch#1249 was mistakenly filed against node-fetch instead of cloudflare - we don't allow constructing GET requests with a content-length in Cloudflare Workers. This change would allow workers to proxy GET requests that originally came from httprb. (Technically, we allow proxying those requests, but not constructing them; but it's common to do |
…ithout a BODY The HTTP spec says (https://tools.ietf.org/html/rfc7230#section-3.3.2): > A user agent SHOULD NOT send a Content-Length header field when the request message does not contain a payload body and the method semantics do not anticipate such a body
c978d25 to
1cd3dd2
Compare
@jyn514 Thank you for clarifying that. I've been stuck in the middle working with a software that sends HTTP requests with this library that are being rejected by Cloudflare workers written by another 3rd party who aren't helping me at all. The assumption that the workers were using Node Fetch was on my part due to the difficulty in getting support. (When I reached out to CloudFlare, they also refused to help me since I don't own the workers in question). As for what the RFC says, you can read more here: #487 But basically, "SHOULD NOT" and "MUST NOT" are different in RFC-speak. httprb is "bending" the spec by doing something not recommended, but it is not "breaking" the spec. And while I've made a case in the link above for httprb to stop "bending" the spec and avoid doing things which are not recommended, I would also make a case to you that CloudFlare is wrong in outright blocking a behavior which is not explicitly forbidden in the spec. Sending a request body is forbidden, but sending a content-length header is not. httprb isn't the only HTTP client I've found which includes a |
|
Thanks! |
| return if @headers[Headers::CONTENT_LENGTH] || chunked? || ( | ||
| @body.source.nil? && %w[GET HEAD DELETE CONNECT].any? do |method| | ||
| @request_header[0].start_with?("#{method} ") | ||
| end | ||
| ) |
There was a problem hiding this comment.
I guess I'm too late, but I think this would look easier to read:
return if @headers[Headers::CONTENT_LENGTH] || chunked?
content_length = @body.size
return if content_length.zero? && (
%w[GET HEAD DELETE CONNECT].any? do |method|
@request_header[0].start_with?("#{method} ")
end
)
@request_header << "#{Headers::CONTENT_LENGTH}: #{@body.size}"Also, seem like it will be more efficient to pass request object into writer for this o-O
There was a problem hiding this comment.
@ixti yeah, I thought the logic here could be better, although I didn't consider that and it seems nice.
It hasn't been released yet so....
There was a problem hiding this comment.
Checking @body.size is not correct, it won't send a Content-Length when the body is an empty string. The other changes seem fine though :)
Changelog: ## [5.3.1] - 2025-06-09 ### Changed - Revert switch to the native llhttp on MRI, as it's not compatible with standalone bundles ([#802](httprb/http#802)) ## [5.3.0] - 2025-06-09 ### Added - (backported) Add .retriable feature to Http - (backported) Add more specific ConnectionError classes - (backported) New feature: RaiseError ### Changed - (backported) Drop depenency on base64 - (backported) Cache header normalization to reduce object allocation - (backported) Use native llhttp on MRI ## [5.2.0] - 2024-02-05 ### Added - Add `Connection#finished_request?` ([#743](httprb/http#743)) - Add `Instrumentation#on_error` ([#746](httprb/http#746)) - Add `base64` dependency (suppresses warnings on Ruby 3.0) ([#759](httprb/http#759)) - Add `PURGE` HTTP verb ([#757](httprb/http#757)) - Add Ruby-3.3 support ### Changed - **BREAKING** Process features in reverse order ([#766](httprb/http#766)) - **BREAKING** Downcase Content-Type charset name ([#753](httprb/http#753)) - **BREAKING** Make URI normalization more conservative ([#758](httprb/http#758)) ### Fixed - Close sockets on initialize failure ([#762](httprb/http#762)) - Prevent CRLF injection due to broken URL normalizer ([#765](httprb/http#765)) [unreleased]: httprb/http@v5.3.0...5-x-stable [5.3.0]: httprb/http@v5.2.0...v5.3.0 [5.2.0]: httprb/http@v5.1.1...v5.2.0 ## 5.1.1 (2022-12-17) * [#731](httprb/http#731) Strip brackets from IPv6 addresses in `HTTP::URI`. ([@jeraki]) * [#722](httprb/http#722) Add `on_redirect` callback. ([@benubois]) ## 5.1.0 (2022-06-17) * Drop ruby-2.5 support. * [#715](httprb/http#715) Set default encoding to UTF-8 for `application/json`. ([@drwl]) * [#712](httprb/http#712) Recognize cookies set by redirect. ([@tkellogg]) * [#707](httprb/http#707) Distinguish connection timeouts. ([@YuLeven]) ## 5.0.4 (2021-10-07) * [#698](httprb/http#698) Fix `HTTP::Timeout::Global#connect_ssl`. ([@tarcieri]) ## 5.0.3 (2021-10-06) * [#695](httprb/http#695) Revert DNS resolving feature. ([@PhilCoggins]) * [#694](httprb/http#694) Fix cookies extraction. ([@flosacca]) ## 5.0.2 (2021-09-10) * [#686](httprb/http#686) Correctly reset the parser. ([@bryanp]) * [#684](httprb/http#684) Don't set Content-Length for GET, HEAD, DELETE, or CONNECT requests without a BODY. ([@jyn514]) * [#679](httprb/http#679) Use features on redirected requests. ([@nomis]) * [#678](httprb/http#678) Restore `HTTP::Response` `:uri` option for backwards compatibility. ([@schwern]) * [#676](httprb/http#676) Update addressable because of CVE-2021-32740. ([@matheussilvasantos]) * [#653](httprb/http#653) Avoid force encodings on frozen strings. ([@bvicenzo]) * [#638](httprb/http#638) DNS failover handling. ([@midnight-wonderer]) ## 5.0.1 (2021-06-26) * [#670](httprb/http#670) Revert `Response#parse` behavior introduced in [#540]. ([@DannyBen]) * [#669](httprb/http#669) Prevent bodies from being resubmitted when following unsafe redirects. ([@odinhb]) * [#664](httprb/http#664) Bump llhttp-ffi to 0.3.0. ([@bryanp]) ## 5.0.0 (2021-05-12) * [#656](httprb/http#656) Handle connection timeouts in `Features` ([@semenyukdmitry]) * [#651](httprb/http#651) Replace `http-parser` with `llhttp` ([@bryanp]) * [#647](httprb/http#647) Add support for `MKCALENDAR` HTTP verb ([@meanphil]) * [#632](httprb/http#632) Respect the SSL context's `verify_hostname` value ([@colemannugent]) * [#625](httprb/http#625) Fix inflator with empty responses ([@LukaszMaslej]) * [#599](httprb/http#599) Allow passing `HTTP::FormData::{Multipart,UrlEncoded}` object directly. ([@ixti]) * [#593](httprb/http#593) [#592](httprb/http#592) Support informational (1XX) responses. ([@ixti]) * [#590](httprb/http#590) [#589](httprb/http#589) Fix response headers paring. ([@Bonias]) * [#587](httprb/http#587) [#585](httprb/http#585) Fix redirections when server responds with multiple Location headers. ([@ixti]) * [#581](httprb/http#581) [#582](httprb/http#582) Add Ruby 2.7.x support. ([@janko]) * [#577](httprb/http#577) Fix `Chainable#timeout` with frozen Hash. ([@antonvolkoff]) * [#576](httprb/http#576) [#524](httprb/http#524) **BREAKING CHANGE** Preserve header names casing. ([@joshuaflanagan]) * [#540](httprb/http#540) [#538](httprb/http#538) **BREAKING CHANGE** Require explicit MIME type for Response#parse ([@ixti]) * [#532](httprb/http#532) Fix pipes support in request bodies. ([@ixti]) * [#530](httprb/http#530) Improve header fields name/value validation. ([@Bonias]) * [#506](httprb/http#506) [#521](httprb/http#521) Skip auto-deflate when there is no body. ([@Bonias]) * [#489](httprb/http#489) Fix HTTP parser. ([@ixti], [@fxposter]) * [#546](httprb/http#546) **BREAKING CHANGE** Provide initiating `HTTP::Request` object on `HTTP::Response`. ([@joshuaflanagan]) * [#571](httprb/http#571) Drop Ruby 2.3.x support. ([@ixti]) * [3ed0c31](httprb/http@3ed0c31) Drop Ruby 2.4.x support.
The HTTP spec says (https://tools.ietf.org/html/rfc7230#section-3.3.2):
Fixes #487.
This is my first time writing Ruby so hopefully the code isn't too bad 😅 happy to make changes.