From b4aed0d4191eb45778f2db0104d9cec0685d065d Mon Sep 17 00:00:00 2001 From: David Humphrey Date: Sun, 4 Oct 2020 16:26:44 -0400 Subject: [PATCH] Deprecated API usage (#2575) (#2594) * Add request.socket, deprecate request.connection * Add deprecation warning FSTDEP005 for request.connection on node 13.0.0+ * Switch to use request.socket internally * Update tests to use .socket * Update docs * Update types to deprecate connection, add socket * Bump light-my-request to 4.1.0 --- docs/Logging.md | 2 +- docs/Request.md | 4 +++- docs/Routes.md | 2 +- lib/logger.js | 2 +- lib/request.js | 15 ++++++++++++--- lib/warnings.js | 3 +++ package.json | 2 +- test/internals/request.test.js | 13 ++++++++----- test/logger.test.js | 2 +- test/types/request.test-d.ts | 2 +- types/request.d.ts | 4 +++- 11 files changed, 35 insertions(+), 16 deletions(-) diff --git a/docs/Logging.md b/docs/Logging.md index b4747d8d61..df57b50949 100644 --- a/docs/Logging.md +++ b/docs/Logging.md @@ -159,7 +159,7 @@ const fastify = Fastify({ headers: request.headers, hostname: request.hostname, remoteAddress: request.ip, - remotePort: request.connection.remotePort + remotePort: request.socket.remotePort } } } diff --git a/docs/Request.md b/docs/Request.md index 193f3996ce..967faa5fdc 100644 --- a/docs/Request.md +++ b/docs/Request.md @@ -20,7 +20,9 @@ Request is a core Fastify object containing the following fields: - `routerMethod` - the method defined for the router that is handling the request - `routerPath` - the path pattern defined for the router that is handling the request - `is404` - true if request is being handled by 404 handler, false if it is not -- `connection` - the underlying connection of the incoming request +- `connection` - Deprecated, use `socket` instead. The underlying connection of the incoming request. +- `socket` - the underlying connection of the incoming request + ```js fastify.post('/:params', options, function (request, reply) { diff --git a/docs/Routes.md b/docs/Routes.md index 1a423fcbe0..a7efae72e4 100644 --- a/docs/Routes.md +++ b/docs/Routes.md @@ -360,7 +360,7 @@ const fastify = Fastify({ headers: req.headers, hostname: req.hostname, remoteAddress: req.ip, - remotePort: req.connection.remotePort + remotePort: req.socket.remotePort } } } diff --git a/lib/logger.js b/lib/logger.js index d462c39d61..fc647bf705 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -53,7 +53,7 @@ const serializers = { version: req.headers['accept-version'], hostname: req.hostname, remoteAddress: req.ip, - remotePort: req.connection.remotePort + remotePort: req.socket.remotePort } }, err: pino.stdSerializers.err, diff --git a/lib/request.js b/lib/request.js index 2c1f22f9a5..aff9385754 100644 --- a/lib/request.js +++ b/lib/request.js @@ -1,6 +1,7 @@ 'use strict' const proxyAddr = require('proxy-addr') +const semver = require('semver') const warning = require('./warnings') function Request (id, params, req, query, log, context) { @@ -87,7 +88,7 @@ function buildRequestWithTrustProxy (R, trustProxy) { const lastIndex = proto.lastIndexOf(',') return lastIndex === -1 ? proto.trim() : proto.slice(lastIndex + 1).trim() } - return this.connection.encrypted ? 'https' : 'http' + return this.socket.encrypted ? 'https' : 'http' } } }) @@ -129,12 +130,20 @@ Object.defineProperties(Request.prototype, { }, connection: { get () { + if (semver.gte(process.versions.node, '13.0.0')) { + warning.emit('FSTDEP005') + } return this.raw.connection } }, + socket: { + get () { + return this.raw.socket + } + }, ip: { get () { - return this.connection.remoteAddress + return this.socket.remoteAddress } }, hostname: { @@ -144,7 +153,7 @@ Object.defineProperties(Request.prototype, { }, protocol: { get () { - return this.connection.encrypted ? 'https' : 'http' + return this.socket.encrypted ? 'https' : 'http' } }, headers: { diff --git a/lib/warnings.js b/lib/warnings.js index c5c9e3ca1a..db2ab5989f 100644 --- a/lib/warnings.js +++ b/lib/warnings.js @@ -8,6 +8,7 @@ const warning = require('fastify-warning')() * - FSTDEP002 * - FSTDEP003 * - FSTDEP004 + * - FSTDEP005 */ warning.create('FastifyDeprecation', 'FSTDEP001', 'You are accessing the Node.js core request object via "request.req", Use "request.raw" instead.') @@ -18,4 +19,6 @@ warning.create('FastifyDeprecation', 'FSTDEP003', 'You are using the legacy Cont warning.create('FastifyDeprecation', 'FSTDEP004', 'You are using the legacy preParsing hook signature. Use the one suggested in the documentation instead.') +warning.create('FastifyDeprecation', 'FSTDEP005', 'You are accessing the deprecated "request.connection" property. Use "request.socket" instead.') + module.exports = warning diff --git a/package.json b/package.json index da174839e8..a3b1a84e71 100644 --- a/package.json +++ b/package.json @@ -176,7 +176,7 @@ "fastify-warning": "^0.2.0", "find-my-way": "^3.0.0", "flatstr": "^1.0.12", - "light-my-request": "^4.0.2", + "light-my-request": "^4.1.0", "pino": "^6.2.1", "proxy-addr": "^2.0.5", "readable-stream": "^3.4.0", diff --git a/test/internals/request.test.js b/test/internals/request.test.js index a4c9676497..da99b7e8eb 100644 --- a/test/internals/request.test.js +++ b/test/internals/request.test.js @@ -12,7 +12,7 @@ test('Regular request', t => { const req = { method: 'GET', url: '/', - connection: { remoteAddress: 'ip' }, + socket: { remoteAddress: 'ip' }, headers } const request = new Request('id', 'params', req, 'query', 'log') @@ -29,7 +29,7 @@ test('Regular request', t => { t.strictEqual(request.body, null) t.strictEqual(request.method, 'GET') t.strictEqual(request.url, '/') - t.deepEqual(request.connection, req.connection) + t.deepEqual(request.socket, req.socket) }) test('Regular request - hostname from authority', t => { @@ -40,7 +40,7 @@ test('Regular request - hostname from authority', t => { const req = { method: 'GET', url: '/', - connection: { remoteAddress: 'ip' }, + socket: { remoteAddress: 'ip' }, headers } @@ -58,7 +58,7 @@ test('Regular request - host header has precedence over authority', t => { const req = { method: 'GET', url: '/', - connection: { remoteAddress: 'ip' }, + socket: { remoteAddress: 'ip' }, headers } const request = new Request('id', 'params', req, 'query', 'log') @@ -75,6 +75,9 @@ test('Request with trust proxy', t => { const req = { method: 'GET', url: '/', + // Some dependencies (proxy-addr, forwarded) still depend on the deprecated + // .connection property, we use .socket. Include both to satisfy everyone. + socket: { remoteAddress: 'ip' }, connection: { remoteAddress: 'ip' }, headers } @@ -94,7 +97,7 @@ test('Request with trust proxy', t => { t.strictEqual(request.body, null) t.strictEqual(request.method, 'GET') t.strictEqual(request.url, '/') - t.deepEqual(request.connection, req.connection) + t.deepEqual(request.socket, req.socket) }) test('Request with trust proxy - no x-forwarded-host header', t => { diff --git a/test/logger.test.js b/test/logger.test.js index 447660eee9..3f012c5533 100644 --- a/test/logger.test.js +++ b/test/logger.test.js @@ -1423,7 +1423,7 @@ test('should redact the authorization header if so specified', t => { headers: req.headers, hostname: req.hostname, remoteAddress: req.ip, - remotePort: req.connection.remotePort + remotePort: req.socket.remotePort } } } diff --git a/test/types/request.test-d.ts b/test/types/request.test-d.ts index f675c8dfab..5d048027d9 100644 --- a/test/types/request.test-d.ts +++ b/test/types/request.test-d.ts @@ -53,7 +53,7 @@ const getHandler: RouteHandler = function (request, _reply) { expectType(request.query) expectType(request.id) expectType(request.log) - expectType(request.connection) + expectType(request.socket) expectType(request.validationError) } diff --git a/types/request.d.ts b/types/request.d.ts index d31db8cec2..41241e7520 100644 --- a/types/request.d.ts +++ b/types/request.d.ts @@ -42,7 +42,9 @@ export interface FastifyRequest< readonly routerPath: string; readonly routerMethod: string; readonly is404: boolean; + readonly socket: RawRequest['socket']; - // `connection` is a deprecated alias for `socket` and doesn't exist in `Http2ServerRequest` + // Prefer `socket` over deprecated `connection` property in node 13.0.0 or higher + // @deprecated readonly connection: RawRequest['socket']; }