From 18013faa9555ebcb1e788497f4d242adbd3b9852 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Mon, 22 Apr 2024 00:06:10 +0200 Subject: [PATCH 1/8] update method matching implementation --- lib/handleRequest.js | 30 +++++++++++--------------- lib/httpMethods.js | 48 ++++++++++++++++++++++++++++-------------- test/delete.test.js | 20 ------------------ test/helper.js | 6 +----- test/lock.test.js | 1 + test/propfind.test.js | 6 ++++-- test/proppatch.test.js | 5 +++-- 7 files changed, 53 insertions(+), 63 deletions(-) diff --git a/lib/handleRequest.js b/lib/handleRequest.js index a29bf1e3f2..ee97d1bb23 100644 --- a/lib/handleRequest.js +++ b/lib/handleRequest.js @@ -1,5 +1,6 @@ 'use strict' +const { bodylessMethods, bodyMethods } = require('./httpMethods') const { validate: validateSchema } = require('./validation') const { preValidationHookRunner, preHandlerHookRunner } = require('./hooks') const wrapThenable = require('./wrapThenable') @@ -20,41 +21,34 @@ function handleRequest (err, request, reply) { const headers = request.headers const context = request[kRouteContext] - if (method === 'GET' || method === 'HEAD') { + if (bodylessMethods.has(method)) { handler(request, reply) return } const contentType = headers['content-type'] - if (method === 'POST' || method === 'PUT' || method === 'PATCH' || method === 'TRACE' || method === 'SEARCH') { + if (bodyMethods.has(method)) { if (contentType === undefined) { if ( headers['transfer-encoding'] === undefined && - (headers['content-length'] === '0' || headers['content-length'] === undefined) - ) { // Request has no body to parse + (headers['content-length'] === undefined || headers['content-length'] === '0') + ) { + // Request has no body to parse handler(request, reply) } else { context.contentTypeParser.run('', handler, request, reply) } } else { - context.contentTypeParser.run(contentType, handler, request, reply) - } - return - } + if (method === 'OPTIONS' && headers['content-length'] === undefined) { + // OPTIONS can have a Content-Type header without a body + handler(request, reply) + return + } - if (method === 'OPTIONS' || method === 'DELETE') { - if ( - contentType !== undefined && - ( - headers['transfer-encoding'] !== undefined || - headers['content-length'] !== undefined - ) - ) { context.contentTypeParser.run(contentType, handler, request, reply) - } else { - handler(request, reply) } + return } diff --git a/lib/httpMethods.js b/lib/httpMethods.js index f32abbaa09..131ea0e78c 100644 --- a/lib/httpMethods.js +++ b/lib/httpMethods.js @@ -1,22 +1,38 @@ 'use strict' +const bodylessMethods = new Set([ + // Standard + 'GET', + 'HEAD', + 'TRACE', + + // WebDAV + 'UNLOCK' +]) + +const bodyMethods = new Set([ + // Standard + 'DELETE', + 'OPTIONS', + 'PATCH', + 'PUT', + 'POST', + + // WebDAV + 'COPY', + 'LOCK', + 'MOVE', + 'MKCOL', + 'PROPFIND', + 'PROPPATCH', + 'SEARCH' +]) + module.exports = { + bodylessMethods, + bodyMethods, supportedMethods: [ - 'DELETE', - 'GET', - 'HEAD', - 'PATCH', - 'POST', - 'PUT', - 'OPTIONS', - 'PROPFIND', - 'PROPPATCH', - 'MKCOL', - 'COPY', - 'MOVE', - 'LOCK', - 'UNLOCK', - 'TRACE', - 'SEARCH' + ...bodylessMethods, + ...bodyMethods ] } diff --git a/test/delete.test.js b/test/delete.test.js index 74f14d269d..e6c5c257dc 100644 --- a/test/delete.test.js +++ b/test/delete.test.js @@ -299,23 +299,3 @@ fastify.listen({ port: 0 }, err => { }) }) }) - -// https://github.com/fastify/fastify/issues/936 -test('shorthand - delete with application/json Content-Type header and without body', t => { - t.plan(4) - const fastify = require('..')() - fastify.delete('/', {}, (req, reply) => { - t.equal(req.body, undefined) - reply.send(req.body) - }) - fastify.inject({ - method: 'DELETE', - url: '/', - headers: { 'Content-Type': 'application/json' }, - body: null - }, (err, response) => { - t.error(err) - t.equal(response.statusCode, 200) - t.same(response.payload.toString(), '') - }) -}) diff --git a/test/helper.js b/test/helper.js index aa18f33e80..f3aeecc0d4 100644 --- a/test/helper.js +++ b/test/helper.js @@ -216,11 +216,7 @@ module.exports.payloadMethod = function (method, t, isSetErrorHandler = false) { }, (err, response, body) => { t.error(err) - if (upMethod === 'OPTIONS') { - t.equal(response.statusCode, 200) - } else { - t.equal(response.statusCode, 415) - } + t.equal(response.statusCode, 415) }) }) diff --git a/test/lock.test.js b/test/lock.test.js index 2875588616..54772d3783 100644 --- a/test/lock.test.js +++ b/test/lock.test.js @@ -55,6 +55,7 @@ fastify.listen({ port: 0 }, err => { t.plan(3) sget({ url: `http://localhost:${fastify.server.address().port}/test/a.txt`, + headers: { 'content-type': 'text/plain' }, body: ` diff --git a/test/propfind.test.js b/test/propfind.test.js index 1b2bdfc878..83798a0f77 100644 --- a/test/propfind.test.js +++ b/test/propfind.test.js @@ -79,7 +79,8 @@ fastify.listen({ port: 0 }, err => { t.plan(3) sget({ url: `http://localhost:${fastify.server.address().port}/test`, - method: 'PROPFIND' + method: 'PROPFIND', + headers: { 'content-type': 'text/plain' } }, (err, response, body) => { t.error(err) t.equal(response.statusCode, 207) @@ -98,7 +99,8 @@ fastify.listen({ port: 0 }, err => { `, - method: 'PROPFIND' + method: 'PROPFIND', + headers: { 'content-type': 'text/plain' } }, (err, response, body) => { t.error(err) t.equal(response.statusCode, 207) diff --git a/test/proppatch.test.js b/test/proppatch.test.js index b4886eef52..7939cf8153 100644 --- a/test/proppatch.test.js +++ b/test/proppatch.test.js @@ -51,6 +51,8 @@ fastify.listen({ port: 0 }, err => { t.plan(3) sget({ url: `http://localhost:${fastify.server.address().port}/test/a.txt`, + method: 'PROPPATCH', + headers: { 'content-type': 'text/plain' }, body: ` @@ -67,8 +69,7 @@ fastify.listen({ port: 0 }, err => { - `, - method: 'PROPPATCH' + ` }, (err, response, body) => { t.error(err) t.equal(response.statusCode, 207) From b9af9081a90395d7a18d71feb1a90258ca936d66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Mon, 22 Apr 2024 00:08:37 +0200 Subject: [PATCH 2/8] push back string comparison --- lib/handleRequest.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/handleRequest.js b/lib/handleRequest.js index ee97d1bb23..9babf696ee 100644 --- a/lib/handleRequest.js +++ b/lib/handleRequest.js @@ -26,13 +26,14 @@ function handleRequest (err, request, reply) { return } - const contentType = headers['content-type'] - if (bodyMethods.has(method)) { + const contentType = headers['content-type'] + const contentLength = headers['content-length'] + if (contentType === undefined) { if ( headers['transfer-encoding'] === undefined && - (headers['content-length'] === undefined || headers['content-length'] === '0') + (contentLength === undefined || contentLength === '0') ) { // Request has no body to parse handler(request, reply) @@ -40,7 +41,7 @@ function handleRequest (err, request, reply) { context.contentTypeParser.run('', handler, request, reply) } } else { - if (method === 'OPTIONS' && headers['content-length'] === undefined) { + if (contentLength === undefined && method === 'OPTIONS') { // OPTIONS can have a Content-Type header without a body handler(request, reply) return From 31430360b983b46022e3e85b9e3a65530ea0cbf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Mon, 22 Apr 2024 00:10:21 +0200 Subject: [PATCH 3/8] extract encoding --- lib/handleRequest.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/handleRequest.js b/lib/handleRequest.js index 9babf696ee..e3760bdf09 100644 --- a/lib/handleRequest.js +++ b/lib/handleRequest.js @@ -31,8 +31,10 @@ function handleRequest (err, request, reply) { const contentLength = headers['content-length'] if (contentType === undefined) { + const transferEncoding = headers['transfer-encoding'] + if ( - headers['transfer-encoding'] === undefined && + transferEncoding === undefined && (contentLength === undefined || contentLength === '0') ) { // Request has no body to parse From 44dc0a5742125630c385b7348f94e77790c8c3e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Mon, 22 Apr 2024 03:30:08 +0200 Subject: [PATCH 4/8] add missing method test --- test/method-missing.test.js | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 test/method-missing.test.js diff --git a/test/method-missing.test.js b/test/method-missing.test.js new file mode 100644 index 0000000000..cb42f657c9 --- /dev/null +++ b/test/method-missing.test.js @@ -0,0 +1,25 @@ +const http = require('http') +const { test } = require('tap') +const Fastify = require('../fastify') + +test('missing method from http client', t => { + t.plan(2) + const fastify = Fastify() + + fastify.listen({ port: 3000 }, (err) => { + t.error(err) + + const port = fastify.server.address().port + const req = http.request({ + port, + method: 'REBIND', + path: '/' + }, (res) => { + t.equal(res.statusCode, 404) + t.end() + fastify.close() + }) + + req.end() + }) +}) From 97ede717084e8c7af755e9f4772c75b64f09adb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Mon, 22 Apr 2024 10:18:13 +0200 Subject: [PATCH 5/8] check for transferEncoding as well --- lib/handleRequest.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/handleRequest.js b/lib/handleRequest.js index e3760bdf09..60ae1e0073 100644 --- a/lib/handleRequest.js +++ b/lib/handleRequest.js @@ -29,13 +29,12 @@ function handleRequest (err, request, reply) { if (bodyMethods.has(method)) { const contentType = headers['content-type'] const contentLength = headers['content-length'] + const transferEncoding = headers['transfer-encoding'] if (contentType === undefined) { - const transferEncoding = headers['transfer-encoding'] - if ( - transferEncoding === undefined && - (contentLength === undefined || contentLength === '0') + (contentLength === undefined || contentLength === '0') && + transferEncoding === undefined ) { // Request has no body to parse handler(request, reply) @@ -43,7 +42,7 @@ function handleRequest (err, request, reply) { context.contentTypeParser.run('', handler, request, reply) } } else { - if (contentLength === undefined && method === 'OPTIONS') { + if (contentLength === undefined && transferEncoding === undefined && method === 'OPTIONS') { // OPTIONS can have a Content-Type header without a body handler(request, reply) return From 62174d4d71100178406cfcb656338ab4e5e53bd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Tue, 23 Apr 2024 12:26:17 +0200 Subject: [PATCH 6/8] skip instead of removing test --- test/delete.test.js | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/delete.test.js b/test/delete.test.js index e6c5c257dc..a715e414aa 100644 --- a/test/delete.test.js +++ b/test/delete.test.js @@ -299,3 +299,43 @@ fastify.listen({ port: 0 }, err => { }) }) }) + +test('shorthand - delete with application/json Content-Type header and null body', t => { + t.plan(4) + const fastify = require('..')() + fastify.delete('/', {}, (req, reply) => { + t.equal(req.body, null) + reply.send(req.body) + }) + fastify.inject({ + method: 'DELETE', + url: '/', + headers: { 'Content-Type': 'application/json' }, + body: 'null' + }, (err, response) => { + t.error(err) + t.equal(response.statusCode, 200) + t.same(response.payload.toString(), 'null') + }) +}) + +// https://github.com/fastify/fastify/issues/936 +// Skip this test because this is an invalid request +test('shorthand - delete with application/json Content-Type header and without body', { skip: true }, t => { + t.plan(4) + const fastify = require('..')() + fastify.delete('/', {}, (req, reply) => { + t.equal(req.body, undefined) + reply.send(req.body) + }) + fastify.inject({ + method: 'DELETE', + url: '/', + headers: { 'Content-Type': 'application/json' }, + body: null + }, (err, response) => { + t.error(err) + t.equal(response.statusCode, 200) + t.same(response.payload.toString(), '') + }) +}) From a064f5006b1c31dc86a405f1eecff765109d0d59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Fri, 26 Apr 2024 14:18:17 +0200 Subject: [PATCH 7/8] Update test/delete.test.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Manuel Spigolon Signed-off-by: Gürgün Dayıoğlu --- test/delete.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/delete.test.js b/test/delete.test.js index a715e414aa..1a296fbe8f 100644 --- a/test/delete.test.js +++ b/test/delete.test.js @@ -321,7 +321,7 @@ test('shorthand - delete with application/json Content-Type header and null body // https://github.com/fastify/fastify/issues/936 // Skip this test because this is an invalid request -test('shorthand - delete with application/json Content-Type header and without body', { skip: true }, t => { +test('shorthand - delete with application/json Content-Type header and without body', { skip: 'https://github.com/fastify/fastify/pull/5419' }, t => { t.plan(4) const fastify = require('..')() fastify.delete('/', {}, (req, reply) => { From 3d00de4808df381482c3139533da9be348fc9dcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Fri, 26 Apr 2024 14:18:34 +0200 Subject: [PATCH 8/8] Update test/method-missing.test.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Manuel Spigolon Signed-off-by: Gürgün Dayıoğlu --- test/method-missing.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/method-missing.test.js b/test/method-missing.test.js index cb42f657c9..bf63bd5182 100644 --- a/test/method-missing.test.js +++ b/test/method-missing.test.js @@ -16,7 +16,6 @@ test('missing method from http client', t => { path: '/' }, (res) => { t.equal(res.statusCode, 404) - t.end() fastify.close() })