fix: handle requests with body already parsed (#23)

Author: dylancarlone

middleware/get-payload.js

@@ -0,0 +1,26 @@
+module.exports = getPayload
+
+function getPayload (request) {
+  // If request.body already exists we can stop here
+  // See https://github.com/octokit/webhooks.js/pull/23
+  if (request.body) {
+    return Promise.resolve(request.body)
+  }
+
+  return new Promise((resolve, reject) => {
+    const dataChunks = []
+
+    request.on('error', reject)
+    request.on('data', (chunk) => dataChunks.push(chunk))
+    request.on('end', () => {
+      const data = Buffer.concat(dataChunks).toString()
+      try {
+        resolve(JSON.parse(data))
+      } catch (error) {
+        error.message = 'Invalid JSON'
+        error.status = 400
+        reject(error)
+      }
+    })
+  })
+}

middleware/middleware.js

@@ -2,6 +2,7 @@ module.exports = middleware
 
 const isntWebhook = require('./isnt-webhook')
 const getMissingHeaders = require('./get-missing-headers')
+const getPayload = require('./get-payload')
 const verifyAndReceive = require('./verify-and-receive')
 
 const debug = require('debug')('webhooks:receiver')
@@ -39,42 +40,23 @@ function middleware (state, request, response, next) {
 
   debug(`${eventName} event received (id: ${id})`)
 
-  const dataChunks = []
-  request.on('error', (error) => {
-    response.statusCode = 500
-    response.end(error.toString())
-  })
+  return getPayload(request)
 
-  request.on('data', (chunk) => {
-    dataChunks.push(chunk)
-  })
-
-  request.on('end', () => {
-    const data = Buffer.concat(dataChunks).toString()
-    let payload
-
-    try {
-      payload = JSON.parse(data)
-    } catch (error) {
-      response.statusCode = 400
-      response.end('Invalid JSON')
-      return
-    }
-
-    verifyAndReceive(state, {
-      id: id,
-      name: eventName,
-      payload,
-      signature
+    .then((payload) => {
+      return verifyAndReceive(state, {
+        id: id,
+        name: eventName,
+        payload,
+        signature
+      })
     })
 
-      .then(() => {
-        response.end('ok\n')
-      })
+    .then(() => {
+      response.end('ok\n')
+    })
 
-      .catch(error => {
-        response.statusCode = error.status || 500
-        response.end(error.toString())
-      })
-  })
+    .catch(error => {
+      response.statusCode = error.status || 500
+      response.end(error.toString())
+    })
 }

test/integration/middleware-test.js

@@ -25,13 +25,14 @@ test('Invalid payload', t => {
   const middleware = createMiddleware({ secret: 'mysecret' })
   middleware(requestMock, responseMock)
 
+    .then(() => {
+      t.is(responseMock.statusCode, 400)
+      t.is(responseMock.end.lastCall.arg, 'SyntaxError: Invalid JSON')
+      t.end()
+    })
+
   requestMock.emit('data', Buffer.from('foo'))
   requestMock.emit('end')
-
-  t.is(responseMock.statusCode, 400)
-  t.is(responseMock.end.lastCall.arg, 'Invalid JSON')
-
-  t.end()
 })
 
 test('request error', t => {
@@ -47,11 +48,13 @@ test('request error', t => {
   const middleware = createMiddleware({ secret: 'mysecret' })
   middleware(requestMock, responseMock)
 
-  const error = new Error('oops')
-  requestMock.emit('error', error)
+    .then(() => {
+      t.is(responseMock.statusCode, 500)
+      t.is(responseMock.end.lastCall.arg, 'Error: oops')
 
-  t.is(responseMock.statusCode, 500)
-  t.is(responseMock.end.lastCall.arg, 'Error: oops')
+      t.end()
+    })
 
-  t.end()
+  const error = new Error('oops')
+  requestMock.emit('error', error)
 })

test/integration/server-test.js

@@ -90,6 +90,53 @@ test('POST / with push event payload', (t) => {
     .catch(t.error)
 })
 
+test('POST / with push event payload (request.body already parsed)', (t) => {
+  t.plan(2)
+
+  const api = new Webhooks({ secret: 'mysecret' })
+  const dataChunks = []
+  const server = http.createServer((req, res) => {
+    req.once('data', chunk => dataChunks.push(chunk))
+    req.once('end', () => {
+      req.body = JSON.parse(Buffer.concat(dataChunks).toString())
+      api.middleware(req, res)
+
+      setTimeout(() => {
+        res.statusCode = 500
+        res.end('Middleware timeout')
+      }, 3000)
+    })
+  })
+
+  api.on('push', (event) => {
+    t.is(event.id, '123e4567-e89b-12d3-a456-426655440000')
+  })
+
+  promisify(server.listen.bind(server))(this.port)
+
+    .then(() => {
+      return axios.post(`http://localhost:${this.port}`, pushEventPayload, {
+        headers: {
+          'X-GitHub-Delivery': '123e4567-e89b-12d3-a456-426655440000',
+          'X-GitHub-Event': 'push',
+          'X-Hub-Signature': 'sha1=f4d795e69b5d03c139cc6ea991ad3e5762d13e2f'
+        }
+      })
+    })
+
+    .catch(t.error)
+
+    .then(result => {
+      t.is(result.status, 200)
+    })
+
+    .then(() => {
+      server.close()
+    })
+
+    .catch(t.error)
+})
+
 test('POST / with push event payload (no signature)', (t) => {
   const api = new Webhooks({ secret: 'mysecret' })
   const server = http.createServer(api.middleware)