Skip to content

Commit

Permalink
fix regression on http2-wrapper caused by node.js compatibility impro…
Browse files Browse the repository at this point in the history
…vements on net (#15318)
  • Loading branch information
cirospaciari authored Nov 22, 2024
1 parent 82cb82d commit c04a2d1
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 48 deletions.
49 changes: 26 additions & 23 deletions src/js/node/tls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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,
Expand Down
Binary file modified test/bun.lockb
Binary file not shown.
50 changes: 26 additions & 24 deletions test/js/node/test/parallel/http2-pipe.test.js
Original file line number Diff line number Diff line change
@@ -1,48 +1,50 @@
//#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;
}

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();
Expand All @@ -53,25 +55,25 @@ 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();
done();
});

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();
});
});
Expand Down
12 changes: 11 additions & 1 deletion test/js/node/tls/node-tls-connect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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 => {
Expand Down
89 changes: 89 additions & 0 deletions test/js/third_party/http2-wrapper/http2-wrapper.test.ts
Original file line number Diff line number Diff line change
@@ -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,
},
});
}
});
7 changes: 7 additions & 0 deletions test/js/third_party/http2-wrapper/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "http2-wrapper-test",
"version": "1.0.0",
"dependencies": {
"http2-wrapper": "2.2.1"
}
}
1 change: 1 addition & 0 deletions test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit c04a2d1

Please sign in to comment.