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

if no content-length, do not add the header #1370

Closed
wants to merge 1 commit into from

Conversation

CraigglesO
Copy link

Fixes errors thrown by Cloudflare. If content-length is 0, the request will error out.

@CLAassistant
Copy link

CLAassistant commented Mar 15, 2022

CLA assistant check
All committers have signed the CLA.

@dcr-stripe
Copy link
Contributor

dcr-stripe commented Mar 30, 2022

Thanks for the PR @CraigglesO and sorry for the delay in responding.

I think we'd actually want to do this on a per-method basis.

As per 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.

We shouldn't be setting this header for verbs that don't anticipate a body.

However, for POST, we should keep sending Content-Length even if it is 0.

Any Content-Length field value greater than or equal to zero is
valid

The underlying undici library does this per-method as well: https://github.com/nodejs/undici/blob/cedc7d26f64aaa0571d1af6eaf82c519a1bcc6da/lib/client.js#L1403-L1407

@dcr-stripe
Copy link
Contributor

I put together #1388 to do this on a per-method basis.

@dcr-stripe dcr-stripe closed this Mar 31, 2022
@dcr-stripe
Copy link
Contributor

Closing this out now that #1388 has been merged - thanks again for the initial PR!

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.

3 participants