Skip to content

Commit

Permalink
Deprecated API usage (fastify#2575) (fastify#2594)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
humphd authored Oct 4, 2020
1 parent d406c41 commit b4aed0d
Show file tree
Hide file tree
Showing 11 changed files with 35 additions and 16 deletions.
2 changes: 1 addition & 1 deletion docs/Logging.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ const fastify = Fastify({
headers: request.headers,
hostname: request.hostname,
remoteAddress: request.ip,
remotePort: request.connection.remotePort
remotePort: request.socket.remotePort
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion docs/Request.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion docs/Routes.md
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ const fastify = Fastify({
headers: req.headers,
hostname: req.hostname,
remoteAddress: req.ip,
remotePort: req.connection.remotePort
remotePort: req.socket.remotePort
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 12 additions & 3 deletions lib/request.js
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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'
}
}
})
Expand Down Expand Up @@ -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: {
Expand All @@ -144,7 +153,7 @@ Object.defineProperties(Request.prototype, {
},
protocol: {
get () {
return this.connection.encrypted ? 'https' : 'http'
return this.socket.encrypted ? 'https' : 'http'
}
},
headers: {
Expand Down
3 changes: 3 additions & 0 deletions lib/warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
Expand All @@ -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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 8 additions & 5 deletions test/internals/request.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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 => {
Expand All @@ -40,7 +40,7 @@ test('Regular request - hostname from authority', t => {
const req = {
method: 'GET',
url: '/',
connection: { remoteAddress: 'ip' },
socket: { remoteAddress: 'ip' },
headers
}

Expand All @@ -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')
Expand All @@ -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
}
Expand All @@ -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 => {
Expand Down
2 changes: 1 addition & 1 deletion test/logger.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/types/request.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const getHandler: RouteHandler = function (request, _reply) {
expectType<RequestQuerystringDefault>(request.query)
expectType<any>(request.id)
expectType<FastifyLoggerInstance>(request.log)
expectType<RawRequestDefaultExpression['socket']>(request.connection)
expectType<RawRequestDefaultExpression['socket']>(request.socket)
expectType<Error & { validation: any; validationContext: string } | undefined>(request.validationError)
}

Expand Down
4 changes: 3 additions & 1 deletion types/request.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
}

0 comments on commit b4aed0d

Please sign in to comment.