Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(MockHttpSocket): forward tls socket properties #556

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion src/interceptors/ClientRequest/MockHttpSocket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,28 @@ export class MockHttpSocket extends MockSocket {
}
}

// Forward TLS Socket properties onto this Socket instance
// in the case of a TLS/SSL connection.
if (Reflect.get(socket, 'encrypted')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we check the protocol instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can but that should be the discerning property to tell apart Socket from TLSSocket, per Node.js docs:

This may be used to distinguish TLS sockets from regular net.Socket instances.
Node.js docs / TLS

I don't mind checking the protocol but if the HTTPS request does have a TLSSocket, it's constructed incorrectly.

Is there a particular test case that fails with this check?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it's a weird choice by Node. But if it's official, I'm in.

const tlsProperties = [
'encrypted',
'authorized',
'getProtocol',
'getSession',
'isSessionReused',
]

tlsProperties.forEach((propertyName) => {
Object.defineProperty(this, propertyName, {
enumerable: true,
get: () => {
const value = Reflect.get(socket, propertyName)
return typeof value === 'function' ? value.bind(socket) : value
},
})
})
}

socket
.on('lookup', (...args) => this.emit('lookup', ...args))
.on('connect', () => {
Expand Down Expand Up @@ -331,9 +353,22 @@ export class MockHttpSocket extends MockSocket {
if (this.baseUrl.protocol === 'https:') {
this.emit('secure')
this.emit('secureConnect')

// A single TLS connection is represented by two "session" events.
this.emit('session', Buffer.from('mock-session-renegotiate'))
this.emit(
'session',
this.connectionOptions.session ||
Buffer.from('mock-session-renegotiate')
)
this.emit('session', Buffer.from('mock-session-resume'))

Reflect.set(this, 'encrypted', true)
// The server certificate is not the same as a CA
// passed to the TLS socket connection options.
Reflect.set(this, 'authorized', false)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikicho, note that the .authorized must be set to false. It's false in a bypassed HTTPS communication as well.

It will only be true if the certificate you pass to socket options is the same one that has signed the server you are requesting. That's false in most cases.

Reflect.set(this, 'getProtocol', () => 'TLSv1.3')
Reflect.set(this, 'getSession', () => undefined)
Reflect.set(this, 'isSessionReused', () => false)
}
}

Expand Down
66 changes: 66 additions & 0 deletions test/modules/http/compliance/http-ssl-socket.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/**
* @vitest-environment node
*/
import { it, expect, beforeAll, afterEach, afterAll } from 'vitest'
import https from 'node:https'
import type { TLSSocket } from 'node:tls'
import { DeferredPromise } from '@open-draft/deferred-promise'
import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest'

const interceptor = new ClientRequestInterceptor()

beforeAll(() => {
interceptor.apply()
})

afterEach(() => {
interceptor.removeAllListeners()
})

afterAll(() => {
interceptor.dispose()
})

it('emits a correct TLS Socket instance for a handled HTTPS request', async () => {
interceptor.on('request', ({ request }) => {
request.respondWith(new Response('hello world'))
})

const request = https.get('https://example.com')
const socketPromise = new DeferredPromise<TLSSocket>()
request.on('socket', (socket) => {
socket.on('connect', () => socketPromise.resolve(socket as TLSSocket))
})

const socket = await socketPromise

// Must be a TLS socket.
expect(socket.encrypted).toBe(true)
// The server certificate wasn't signed by one of the CA
// specified in the Socket constructor.
expect(socket.authorized).toBe(false)

expect(socket.getSession()).toBeUndefined()
expect(socket.getProtocol()).toBe('TLSv1.3')
expect(socket.isSessionReused()).toBe(false)
})

it('emits a correct TLS Socket instance for a bypassed HTTPS request', async () => {
const request = https.get('https://example.com')
const socketPromise = new DeferredPromise<TLSSocket>()
request.on('socket', (socket) => {
socket.on('connect', () => socketPromise.resolve(socket as TLSSocket))
})

const socket = await socketPromise

// Must be a TLS socket.
expect(socket.encrypted).toBe(true)
// The server certificate wasn't signed by one of the CA
// specified in the Socket constructor.
expect(socket.authorized).toBe(false)

expect(socket.getSession()).toBeUndefined()
expect(socket.getProtocol()).toBe('TLSv1.3')
expect(socket.isSessionReused()).toBe(false)
})
Loading