Stop sending Content-Length header for GET and DELETE requests that h…

…ave no body.
stripe · Mar 30, 2022 · 5a29284 · 5a29284
1 parent c271647
commit 5a29284
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 4 deletions.
diff --git 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
@@ -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, '{}');