diff --git a/lib/StripeResource.js b/lib/StripeResource.js index c9acf3f41a..208d4382a0 100644 --- a/lib/StripeResource.js +++ b/lib/StripeResource.js @@ -369,7 +369,6 @@ StripeResource.prototype = { Authorization: auth ? `Bearer ${auth}` : this._stripe.getApiField('auth'), Accept: 'application/json', 'Content-Type': 'application/x-www-form-urlencoded', - 'Content-Length': contentLength, 'User-Agent': this._getUserAgentString(), 'X-Stripe-Client-User-Agent': clientUserAgent, 'X-Stripe-Client-Telemetry': this._getTelemetryHeader(), @@ -381,6 +380,34 @@ StripeResource.prototype = { ), }; + // As per https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2: + // A user agent SHOULD send a Content-Length in a request message when + // no Transfer-Encoding is sent and the request method defines a meaning + // for an enclosed payload body. For example, a Content-Length header + // field is normally sent in a POST request even when the value is 0 + // (indicating an empty payload body). 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. + // + // These method types are expected to have bodies and so we should always + // include a Content-Length. + const methodHasPayload = + method == 'POST' || method == 'PUT' || method == 'PATCH'; + + // If a content length was specified, we always include it regardless of + // whether the method semantics anticipate such a body. This keeps us + // consistent with historical behavior. We do however want to warn on this + // and fix these cases as they are semantically incorrect. + if (methodHasPayload || contentLength) { + if (!methodHasPayload) { + utils.emitWarning( + `${method} method had non-zero contentLength but no payload is expected for this verb` + ); + } + defaultHeaders['Content-Length'] = contentLength; + } + return Object.assign( utils.removeNullish(defaultHeaders), // If the user supplied, say 'idempotency-key', override instead of appending by ensuring caps are the same. diff --git a/test/StripeResource.spec.js b/test/StripeResource.spec.js index e84937cc5f..d608a0d057 100644 --- a/test/StripeResource.spec.js +++ b/test/StripeResource.spec.js @@ -75,7 +75,13 @@ describe('StripeResource', () => { data, }; - const scope = nock(`https://${options.host}`) + const scope = nock( + `https://${options.host}`, + // No Content-Length should be present for GET requests. + { + badheaders: ['Content-Length'], + } + ) .get( `${options.path}?customer=cus_123&subscription_items[0][plan]=foo&subscription_items[0][quantity]=2&subscription_items[1][id]=si_123&subscription_items[1][deleted]=true`, '' @@ -116,7 +122,13 @@ describe('StripeResource', () => { }; const host = stripe.getConstant('DEFAULT_HOST'); - const scope = nock(`https://${host}`) + const scope = nock( + `https://${host}`, + // No Content-Length should be present for DELETE requests. + { + badheaders: ['Content-Length'], + } + ) .delete(/.*/) .reply(200, '{}'); @@ -141,7 +153,42 @@ describe('StripeResource', () => { 'customer=cus_123&items[0][plan]=foo&items[0][quantity]=2&items[1][id]=si_123&items[1][deleted]=true', }; - const scope = nock(`https://${options.host}`) + const scope = nock( + `https://${options.host}`, + // Content-Length should be present for POST. + { + reqheaders: {'Content-Length': options.body.length}, + } + ) + .post(options.path, options.body) + .reply(200, '{}'); + + realStripe.subscriptions.update( + 'sub_123', + options.data, + (err, response) => { + done(err); + scope.done(); + } + ); + }); + + it('always includes Content-Length in POST requests even when empty', (done) => { + const options = { + host: stripe.getConstant('DEFAULT_HOST'), + path: '/v1/subscriptions/sub_123', + data: {}, + body: '', + }; + + const scope = nock( + `https://${options.host}`, + // Content-Length should be present for POST even when the body is + // empty. + { + reqheaders: {'Content-Length': 0}, + } + ) .post(options.path, options.body) .reply(200, '{}');