fix regression on http2-wrapper caused by node.js compatibility improvements on net (#15318)

This commit is contained in:
Ciro Spaciari
2024-11-22 20:22:35 -03:00
committed by GitHub
parent 82cb82d828
commit c04a2d1dfc
7 changed files with 160 additions and 48 deletions

View File

@@ -2,6 +2,8 @@
const { isArrayBufferView, isTypedArray } = require("node:util/types");
const { addServerName } = require("../internal/net");
const net = require("node:net");
const { Duplex } = require("node:stream");
const { Server: NetServer, [Symbol.for("::bunternal::")]: InternalTCPSocket } = net;
const { rootCertificates, canonicalizeIP } = $cpp("NodeTLS.cpp", "createNodeTLSBinding");
@@ -326,21 +328,22 @@ const TLSSocket = (function (InternalTLSSocket) {
class TLSSocket extends InternalTCPSocket {
#secureContext;
ALPNProtocols;
#socket;
#checkServerIdentity;
#session;
alpnProtocol = null;
constructor(socket, options) {
super(socket instanceof InternalTCPSocket ? options : options || socket);
super(socket instanceof InternalTCPSocket || socket instanceof Duplex ? options : options || socket);
options = options || socket || {};
if (typeof options === "object") {
const { ALPNProtocols } = options;
if (ALPNProtocols) {
convertALPNProtocols(ALPNProtocols, this);
}
if (socket instanceof InternalTCPSocket) {
this.#socket = socket;
if (socket instanceof InternalTCPSocket || socket instanceof Duplex) {
this._handle = socket;
// keep compatibility with http2-wrapper or other places that try to grab JSStreamSocket in node.js, with here is just the TLSSocket
this._handle._parentWrap = this;
}
}
@@ -373,31 +376,31 @@ const TLSSocket = (function (InternalTLSSocket) {
}
getSession() {
return this._handle?.getSession();
return this._handle?.getSession?.();
}
getEphemeralKeyInfo() {
return this._handle?.getEphemeralKeyInfo();
return this._handle?.getEphemeralKeyInfo?.();
}
getCipher() {
return this._handle?.getCipher();
return this._handle?.getCipher?.();
}
getSharedSigalgs() {
return this._handle?.getSharedSigalgs();
return this._handle?.getSharedSigalgs?.();
}
getProtocol() {
return this._handle?.getTLSVersion();
return this._handle?.getTLSVersion?.();
}
getFinished() {
return this._handle?.getTLSFinishedMessage() || undefined;
return this._handle?.getTLSFinishedMessage?.() || undefined;
}
getPeerFinished() {
return this._handle?.getTLSPeerFinishedMessage() || undefined;
return this._handle?.getTLSPeerFinishedMessage?.() || undefined;
}
isSessionReused() {
return !!this.#session;
@@ -424,13 +427,13 @@ const TLSSocket = (function (InternalTLSSocket) {
if (options.rejectUnauthorized !== undefined) rejectUnauthorized = !!options.rejectUnauthorized;
if (requestCert !== this._requestCert || rejectUnauthorized !== this._rejectUnauthorized) {
socket.setVerifyMode(requestCert, rejectUnauthorized);
socket.setVerifyMode?.(requestCert, rejectUnauthorized);
this._requestCert = requestCert;
this._rejectUnauthorized = rejectUnauthorized;
}
}
try {
socket.renegotiate();
socket.renegotiate?.();
// if renegotiate is successful should emit secure event when done
typeof callback === "function" && this.once("secure", () => callback(null));
return true;
@@ -444,21 +447,21 @@ const TLSSocket = (function (InternalTLSSocket) {
disableRenegotiation() {
this.#renegotiationDisabled = true;
// disable renegotiation on the socket
return this._handle?.disableRenegotiation();
return this._handle?.disableRenegotiation?.();
}
getTLSTicket() {
return this._handle?.getTLSTicket();
return this._handle?.getTLSTicket?.();
}
exportKeyingMaterial(length, label, context) {
if (context) {
return this._handle?.exportKeyingMaterial(length, label, context);
return this._handle?.exportKeyingMaterial?.(length, label, context);
}
return this._handle?.exportKeyingMaterial(length, label);
return this._handle?.exportKeyingMaterial?.(length, label);
}
setMaxSendFragment(size) {
return this._handle?.setMaxSendFragment(size) || false;
return this._handle?.setMaxSendFragment?.(size) || false;
}
// only for debug purposes so we just mock for now
@@ -472,23 +475,23 @@ const TLSSocket = (function (InternalTLSSocket) {
}
// if the socket is detached we can't set the servername but we set this property so when open will auto set to it
this.servername = name;
this._handle?.setServername(name);
this._handle?.setServername?.(name);
}
setSession(session) {
this.#session = session;
if (typeof session === "string") session = Buffer.from(session, "latin1");
return this._handle?.setSession(session);
return this._handle?.setSession?.(session);
}
getPeerCertificate(abbreviated) {
const cert =
arguments.length < 1 ? this._handle?.getPeerCertificate() : this._handle?.getPeerCertificate(abbreviated);
arguments.length < 1 ? this._handle?.getPeerCertificate?.() : this._handle?.getPeerCertificate?.(abbreviated);
if (cert) {
return translatePeerCertificate(cert);
}
}
getCertificate() {
// need to implement certificate on socket.zig
const cert = this._handle?.getCertificate();
const cert = this._handle?.getCertificate?.();
if (cert) {
// It's not a peer cert, but the formatting is identical.
return translatePeerCertificate(cert);
@@ -503,7 +506,7 @@ const TLSSocket = (function (InternalTLSSocket) {
[buntls](port, host) {
return {
socket: this.#socket,
socket: this._handle,
ALPNProtocols: this.ALPNProtocols,
serverName: this.servername || host || "localhost",
checkServerIdentity: this.#checkServerIdentity,

Binary file not shown.

View File

@@ -1,17 +1,19 @@
//#FILE: test-http2-pipe.js
//#SHA1: bb970b612d495580b8c216a1b202037e5eb0721e
//-----------------
'use strict';
"use strict";
const http2 = require('http2');
const fs = require('fs');
const path = require('path');
const os = require('os');
import { afterEach, beforeEach, test, expect, describe, mock } from "bun:test";
const http2 = require("http2");
const fs = require("fs");
const path = require("path");
const os = require("os");
// Skip the test if crypto is not available
let hasCrypto;
try {
require('crypto');
require("crypto");
hasCrypto = true;
} catch (err) {
hasCrypto = false;
@@ -19,30 +21,30 @@ try {
const testIfCrypto = hasCrypto ? test : test.skip;
describe('HTTP2 Pipe', () => {
describe("HTTP2 Pipe", () => {
let server;
let serverPort;
let tmpdir;
const fixturesDir = path.join(__dirname, '..', 'fixtures');
const loc = path.join(fixturesDir, 'person-large.jpg');
const fixturesDir = path.join(__dirname, "..", "fixtures");
const loc = path.join(fixturesDir, "person-large.jpg");
let fn;
beforeAll(async () => {
tmpdir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'http2-test-'));
fn = path.join(tmpdir, 'http2-url-tests.js');
beforeEach(() => {
tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), "http2-test-"));
fn = path.join(tmpdir, "http2-url-tests.js");
});
afterAll(async () => {
await fs.promises.rm(tmpdir, { recursive: true, force: true });
afterEach(() => {
fs.rmSync(tmpdir, { recursive: true, force: true });
});
testIfCrypto('Piping should work as expected with createWriteStream', (done) => {
testIfCrypto("Piping should work as expected with createWriteStream", done => {
server = http2.createServer();
server.on('stream', (stream) => {
server.on("stream", stream => {
const dest = stream.pipe(fs.createWriteStream(fn));
dest.on('finish', () => {
dest.on("finish", () => {
expect(fs.readFileSync(loc).length).toBe(fs.readFileSync(fn).length);
});
stream.respond();
@@ -53,13 +55,13 @@ describe('HTTP2 Pipe', () => {
serverPort = server.address().port;
const client = http2.connect(`http://localhost:${serverPort}`);
const req = client.request({ ':method': 'POST' });
const req = client.request({ ":method": "POST" });
const responseHandler = jest.fn();
req.on('response', responseHandler);
const responseHandler = mock(() => {});
req.on("response", responseHandler);
req.resume();
req.on('close', () => {
req.on("close", () => {
expect(responseHandler).toHaveBeenCalled();
server.close();
client.close();
@@ -67,11 +69,11 @@ describe('HTTP2 Pipe', () => {
});
const str = fs.createReadStream(loc);
const strEndHandler = jest.fn();
str.on('end', strEndHandler);
const strEndHandler = mock(() => {});
str.on("end", strEndHandler);
str.pipe(req);
req.on('finish', () => {
req.on("finish", () => {
expect(strEndHandler).toHaveBeenCalled();
});
});

View File

@@ -3,6 +3,7 @@ import { tls as COMMON_CERT_ } from "harness";
import net from "net";
import { join } from "path";
import tls, { checkServerIdentity, connect as tlsConnect, TLSSocket } from "tls";
import stream from "stream";
import { Duplex } from "node:stream";
@@ -116,7 +117,16 @@ it("should have checkServerIdentity", async () => {
expect(checkServerIdentity).toBeFunction();
expect(tls.checkServerIdentity).toBeFunction();
});
it("should be able to grab the JSStreamSocket constructor", () => {
// this keep http2-wrapper compatibility with node.js
const socket = new tls.TLSSocket(new stream.PassThrough());
//@ts-ignore
expect(socket._handle).not.toBeNull();
//@ts-ignore
expect(socket._handle._parentWrap).not.toBeNull();
//@ts-ignore
expect(socket._handle._parentWrap.constructor).toBeFunction();
});
for (const { name, connect } of tests) {
describe(name, () => {
it("should work with alpnProtocols", done => {

View File

@@ -0,0 +1,89 @@
import { test, expect } from "bun:test";
import { tls } from "harness";
import http2Wrapper from "http2-wrapper";
import type { AutoRequestOptions } from "http2-wrapper";
import http from "http";
async function doRequest(options: AutoRequestOptions) {
const { promise, resolve, reject } = Promise.withResolvers();
const request = await http2Wrapper.auto(options, (response: http.IncomingMessage) => {
if (response.statusCode !== 200) {
return reject(new Error(`expected status code 200 rejected: ${response.statusCode}`));
}
const body: Array<Buffer> = [];
response.on("error", reject);
response.on("data", (chunk: Buffer) => body.push(chunk));
response.on("end", () => {
resolve(Buffer.concat(body).toString());
});
});
request.on("error", reject);
request.end("123456");
const body = (await promise) as string;
expect(body).toBeString();
const parsed = JSON.parse(body);
expect(parsed.data).toBe("123456");
}
test("should allow http/1.1 when using http2-wrapper", async () => {
{
using server = Bun.serve({
async fetch(req) {
return new Response(
JSON.stringify({
data: await req.text(),
}),
{
headers: {
"content-type": "application/json",
},
},
);
},
});
await doRequest({
host: "localhost",
port: server.port,
protocol: "http:",
path: "/post",
method: "POST",
headers: {
"content-length": 6,
},
});
}
{
using server = Bun.serve({
tls,
hostname: "localhost",
async fetch(req) {
return new Response(
JSON.stringify({
data: await req.text(),
}),
{
headers: {
"content-type": "application/json",
},
},
);
},
});
await doRequest({
host: "localhost",
port: server.port,
protocol: "https:",
path: "/post",
method: "POST",
ca: tls.cert,
headers: {
"content-length": 6,
},
});
}
});

View File

@@ -0,0 +1,7 @@
{
"name": "http2-wrapper-test",
"version": "1.0.0",
"dependencies": {
"http2-wrapper": "2.2.1"
}
}

View File

@@ -26,6 +26,7 @@
"express": "4.18.2",
"fast-glob": "3.3.1",
"filenamify": "6.0.0",
"http2-wrapper": "2.2.1",
"https-proxy-agent": "7.0.5",
"iconv-lite": "0.6.3",
"isbot": "5.1.13",