feat: set undefined on null input (#2731)

* feat: handle empty body.payload as undefined * test: adjust tests to body === undefined * test: add tests for undefined req.body on 204 * docs: update docs for undefined as default
fastify · Dec 16, 2020 · 67a7a19 · 67a7a19
1 parent f6ee355
commit 67a7a19
Show file tree

Hide file tree

Showing 8 changed files with 65 additions and 27 deletions.
diff --git a/docs/Hooks.md b/docs/Hooks.md
@@ -53,7 +53,7 @@ fastify.addHook('onRequest', async (request, reply) => {
 })
 ```
 
-**Notice:** in the [onRequest](#onrequest) hook, `request.body` will always be `null`, because the body parsing happens before the [preValidation](#prevalidation) hook.
+**Notice:** in the [onRequest](#onrequest) hook, `request.body` will always be `undefined`, because the body parsing happens before the [preValidation](#prevalidation) hook.
 
 ### preParsing
 
@@ -78,7 +78,7 @@ fastify.addHook('preParsing', async (request, reply, payload) => {
 })
 ```
 
-**Notice:** in the [preParsing](#preparsing) hook, `request.body` will always be `null`, because the body parsing happens before the [preValidation](#prevalidation) hook.
+**Notice:** in the [preParsing](#preparsing) hook, `request.body` will always be `undefined`, because the body parsing happens before the [preValidation](#prevalidation) hook.
 
 **Notice:** you should also add `receivedEncodedLength` property to the returned stream. This property is used to correctly match the request payload with the `Content-Length` header value. Ideally, this property should be updated on each received chunk.
 

diff --git a/lib/request.js b/lib/request.js
@@ -11,7 +11,7 @@ function Request (id, params, req, query, log, context) {
   this.raw = req
   this.query = query
   this.log = log
-  this.body = null
+  this.body = undefined
 }
 
 function getTrustProxyFn (tp) {
@@ -50,7 +50,7 @@ function buildRegularRequest (R) {
     this.raw = req
     this.query = query
     this.log = log
-    this.body = null
+    this.body = undefined
   }
   _Request.prototype = new R()
 

diff --git a/lib/validation.js b/lib/validation.js
@@ -67,7 +67,8 @@ function compileSchemasForValidation (context, compile) {
 }
 
 function validateParam (validatorFunction, request, paramName) {
-  const ret = validatorFunction && validatorFunction(request[paramName])
+  const isUndefined = request[paramName] === undefined
+  const ret = validatorFunction && validatorFunction(isUndefined ? null : request[paramName])
   if (ret === false) return validatorFunction.errors
   if (ret && ret.error) return ret.error
   if (ret && ret.value) request[paramName] = ret.value

diff --git a/test/delete.test.js b/test/delete.test.js
@@ -302,7 +302,7 @@ test('shorthand - delete with application/json Content-Type header and without b
   t.plan(4)
   const fastify = require('..')()
   fastify.delete('/', {}, (req, reply) => {
-    t.equal(req.body, null)
+    t.equal(req.body, undefined)
     reply.send(req.body)
   })
   fastify.inject({
@@ -313,6 +313,6 @@ test('shorthand - delete with application/json Content-Type header and without b
   }, (err, response) => {
     t.error(err)
     t.strictEqual(response.statusCode, 200)
-    t.deepEqual(JSON.parse(response.payload), null)
+    t.deepEqual(response.payload, '')
   })
 })
diff --git a/test/helper.js b/test/helper.js
@@ -189,7 +189,7 @@ module.exports.payloadMethod = function (method, t, isSetErrorHandler = false) {
       }, (err, response, body) => {
         t.error(err)
         t.strictEqual(response.statusCode, 200)
-        t.strictEqual(JSON.parse(body.toString()), null)
+        t.strictEqual(body.toString(), '')
       })
 
       // Must use inject to make a request without a Content-Length header
@@ -199,7 +199,7 @@ module.exports.payloadMethod = function (method, t, isSetErrorHandler = false) {
       }, (err, res) => {
         t.error(err)
         t.strictEqual(res.statusCode, 200)
-        t.strictEqual(JSON.parse(res.payload), null)
+        t.strictEqual(res.payload, '')
       })
     })
 

diff --git a/test/hooks.test.js b/test/hooks.test.js
@@ -2058,7 +2058,7 @@ test('request in onRequest, preParsing, preValidation and onResponse', t => {
   const fastify = Fastify()
 
   fastify.addHook('onRequest', function (request, reply, done) {
-    t.deepEqual(request.body, null)
+    t.deepEqual(request.body, undefined)
     t.deepEqual(request.query, { key: 'value' })
     t.deepEqual(request.params, { greeting: 'hello' })
     t.deepEqual(request.headers, {
@@ -2072,7 +2072,7 @@ test('request in onRequest, preParsing, preValidation and onResponse', t => {
   })
 
   fastify.addHook('preParsing', function (request, reply, payload, done) {
-    t.deepEqual(request.body, null)
+    t.deepEqual(request.body, undefined)
     t.deepEqual(request.query, { key: 'value' })
     t.deepEqual(request.params, { greeting: 'hello' })
     t.deepEqual(request.headers, {

diff --git a/test/internals/request.test.js b/test/internals/request.test.js
@@ -26,7 +26,7 @@ test('Regular request', t => {
   t.strictEqual(request.ip, 'ip')
   t.strictEqual(request.ips, undefined)
   t.strictEqual(request.hostname, 'hostname')
-  t.strictEqual(request.body, null)
+  t.strictEqual(request.body, undefined)
   t.strictEqual(request.method, 'GET')
   t.strictEqual(request.url, '/')
   t.deepEqual(request.socket, req.socket)
@@ -94,7 +94,7 @@ test('Request with trust proxy', t => {
   t.strictEqual(request.ip, '2.2.2.2')
   t.deepEqual(request.ips, ['ip', '1.1.1.1', '2.2.2.2'])
   t.strictEqual(request.hostname, 'example.com')
-  t.strictEqual(request.body, null)
+  t.strictEqual(request.body, undefined)
   t.strictEqual(request.method, 'GET')
   t.strictEqual(request.url, '/')
   t.deepEqual(request.socket, req.socket)

diff --git a/test/nullable-validation.test.js b/test/nullable-validation.test.js
@@ -61,8 +61,8 @@ test('object or null body', t => {
     method: 'POST',
     url: '/',
     handler: (req, reply) => {
-      t.strictEqual(req.body, null)
-      reply.code(200).send({ requestBody: req.body })
+      t.strictEqual(req.body, undefined)
+      reply.code(200).send({ isUndefinedBody: req.body === undefined })
     },
     schema: {
       body: {
@@ -79,10 +79,8 @@ test('object or null body', t => {
           type: 'object',
           nullable: true,
           properties: {
-            requestBody: {
-              type: 'string',
-              format: 'email',
-              nullable: true
+            isUndefinedBody: {
+              type: 'boolean'
             }
           }
         }
@@ -100,7 +98,7 @@ test('object or null body', t => {
     }, (err, response, body) => {
       t.error(err)
       t.strictEqual(response.statusCode, 200)
-      t.deepEqual(JSON.parse(body), { requestBody: null })
+      t.deepEqual(JSON.parse(body), { isUndefinedBody: true })
     })
   })
 })
@@ -114,8 +112,8 @@ test('nullable body', t => {
     method: 'POST',
     url: '/',
     handler: (req, reply) => {
-      t.strictEqual(req.body, null)
-      reply.code(200).send({ requestBody: req.body })
+      t.strictEqual(req.body, undefined)
+      reply.code(200).send({ isUndefinedBody: req.body === undefined })
     },
     schema: {
       body: {
@@ -133,10 +131,8 @@ test('nullable body', t => {
           type: 'object',
           nullable: true,
           properties: {
-            requestBody: {
-              type: 'string',
-              format: 'email',
-              nullable: true
+            isUndefinedBody: {
+              type: 'boolean'
             }
           }
         }
@@ -154,7 +150,48 @@ test('nullable body', t => {
     }, (err, response, body) => {
       t.error(err)
       t.strictEqual(response.statusCode, 200)
-      t.deepEqual(JSON.parse(body), { requestBody: null })
+      t.deepEqual(JSON.parse(body), { isUndefinedBody: true })
+    })
+  })
+})
+
+test('Nullable body with 204', t => {
+  t.plan(5)
+
+  const fastify = Fastify()
+
+  fastify.route({
+    method: 'POST',
+    url: '/',
+    handler: (req, reply) => {
+      t.strictEqual(req.body, undefined)
+      reply.code(204).send()
+    },
+    schema: {
+      body: {
+        type: 'object',
+        nullable: true,
+        properties: {
+          hello: {
+            type: 'string',
+            format: 'email'
+          }
+        }
+      }
+    }
+  })
+
+  fastify.listen(0, (err) => {
+    fastify.server.unref()
+    t.error(err)
+
+    sget({
+      method: 'POST',
+      url: 'http://localhost:' + fastify.server.address().port
+    }, (err, response, body) => {
+      t.error(err)
+      t.strictEqual(response.statusCode, 204)
+      t.strictEqual(body.length, 0)
     })
   })
 })