From 24bcfb28106e81810bdebaa9e7bfc1abd141ca21 Mon Sep 17 00:00:00 2001 From: Sri Krishna <93153132+srikrsna-buf@users.noreply.github.com> Date: Tue, 7 Nov 2023 18:51:49 +0530 Subject: [PATCH] Don't set `User-Agent` header in connect-web (#912) --- packages/connect-web-bench/README.md | 2 +- packages/connect-web/src/connect-transport.ts | 2 ++ .../connect-web/src/grpc-web-transport.ts | 4 +-- .../protocol-connect/handler-factory.spec.ts | 8 +++--- .../protocol-connect/request-header.spec.ts | 26 +++++++++++++++++-- .../src/protocol-connect/request-header.ts | 7 ++++- .../connect/src/protocol-connect/transport.ts | 2 ++ .../protocol-grpc-web/handler-factory.spec.ts | 4 +-- .../protocol-grpc-web/request-header.spec.ts | 16 ++++++++++-- .../src/protocol-grpc-web/request-header.ts | 13 ++++++++-- .../src/protocol-grpc-web/transport.ts | 2 ++ 11 files changed, 70 insertions(+), 16 deletions(-) diff --git a/packages/connect-web-bench/README.md b/packages/connect-web-bench/README.md index 64dfedb20..619786049 100644 --- a/packages/connect-web-bench/README.md +++ b/packages/connect-web-bench/README.md @@ -10,5 +10,5 @@ it like a web server would usually do. | code generator | bundle size | minified | compressed | |----------------|-------------------:|-----------------------:|---------------------:| -| connect | 115,339 b | 50,669 b | 13,669 b | +| connect | 115,395 b | 50,680 b | 13,669 b | | grpc-web | 414,071 b | 300,352 b | 53,255 b | diff --git a/packages/connect-web/src/connect-transport.ts b/packages/connect-web/src/connect-transport.ts index 0f4a222ef..fea7b6cf7 100644 --- a/packages/connect-web/src/connect-transport.ts +++ b/packages/connect-web/src/connect-transport.ts @@ -177,6 +177,7 @@ export function createConnectTransport( useBinaryFormat, timeoutMs, header, + false, ), contextValues: contextValues ?? createContextValues(), message, @@ -323,6 +324,7 @@ export function createConnectTransport( useBinaryFormat, timeoutMs, header, + false, ), contextValues: contextValues ?? createContextValues(), message: input, diff --git a/packages/connect-web/src/grpc-web-transport.ts b/packages/connect-web/src/grpc-web-transport.ts index 05c0c0e65..fbce76f13 100644 --- a/packages/connect-web/src/grpc-web-transport.ts +++ b/packages/connect-web/src/grpc-web-transport.ts @@ -168,7 +168,7 @@ export function createGrpcWebTransport( redirect: "error", mode: "cors", }, - header: requestHeader(useBinaryFormat, timeoutMs, header), + header: requestHeader(useBinaryFormat, timeoutMs, header, false), contextValues: contextValues ?? createContextValues(), message, }, @@ -327,7 +327,7 @@ export function createGrpcWebTransport( redirect: "error", mode: "cors", }, - header: requestHeader(useBinaryFormat, timeoutMs, header), + header: requestHeader(useBinaryFormat, timeoutMs, header, false), contextValues: contextValues ?? createContextValues(), message: input, }, diff --git a/packages/connect/src/protocol-connect/handler-factory.spec.ts b/packages/connect/src/protocol-connect/handler-factory.spec.ts index 13419b235..c1b58cc3b 100644 --- a/packages/connect/src/protocol-connect/handler-factory.spec.ts +++ b/packages/connect/src/protocol-connect/handler-factory.spec.ts @@ -333,7 +333,7 @@ describe("createHandlerFactory()", function () { httpVersion: "2.0", method: "POST", url: `https://example.com/${service.typeName}/${method.name}`, - header: requestHeader(method.kind, true, timeoutMs, undefined), + header: requestHeader(method.kind, true, timeoutMs, undefined, true), body: createAsyncIterable([new Uint8Array(0)]), signal: new AbortController().signal, }); @@ -384,7 +384,7 @@ describe("createHandlerFactory()", function () { httpVersion: "2.0", method: "POST", url: `https://example.com/${service.typeName}/${method.name}`, - header: requestHeader(method.kind, true, timeoutMs, undefined), + header: requestHeader(method.kind, true, timeoutMs, undefined, true), body: createAsyncIterable([encodeEnvelope(0, new Uint8Array(0))]), signal: new AbortController().signal, }); @@ -494,7 +494,7 @@ describe("createHandlerFactory()", function () { httpVersion: "2.0", method: "POST", url: `https://example.com/${service.typeName}/${method.name}`, - header: requestHeader(method.kind, true, undefined, undefined), + header: requestHeader(method.kind, true, undefined, undefined, true), body: createAsyncIterable([new Uint8Array(0)]), signal: ac.signal, }); @@ -525,7 +525,7 @@ describe("createHandlerFactory()", function () { httpVersion: "2.0", method: "POST", url: `https://example.com/${service.typeName}/${method.name}`, - header: requestHeader(method.kind, true, undefined, undefined), + header: requestHeader(method.kind, true, undefined, undefined, true), body: createAsyncIterable([encodeEnvelope(0, new Uint8Array(0))]), signal: ac.signal, }); diff --git a/packages/connect/src/protocol-connect/request-header.spec.ts b/packages/connect/src/protocol-connect/request-header.spec.ts index 395fe004e..9ac819f84 100644 --- a/packages/connect/src/protocol-connect/request-header.spec.ts +++ b/packages/connect/src/protocol-connect/request-header.spec.ts @@ -33,7 +33,13 @@ function listHeaderKeys(header: Headers): string[] { describe("requestHeader", () => { it("should create request headers", () => { - const headers = requestHeader(MethodKind.Unary, true, undefined, undefined); + const headers = requestHeader( + MethodKind.Unary, + true, + undefined, + undefined, + true, + ); expect(listHeaderKeys(headers)).toEqual([ "connect-protocol-version", "content-type", @@ -45,7 +51,7 @@ describe("requestHeader", () => { }); it("should create request headers with timeout", () => { - const headers = requestHeader(MethodKind.Unary, true, 10, undefined); + const headers = requestHeader(MethodKind.Unary, true, 10, undefined, true); expect(listHeaderKeys(headers)).toEqual([ "connect-protocol-version", "connect-timeout-ms", @@ -54,6 +60,20 @@ describe("requestHeader", () => { ]); expect(headers.get("Connect-Timeout-Ms")).toBe("10"); }); + + it("should exclude user-agent", () => { + const headers = requestHeader( + MethodKind.Unary, + true, + undefined, + undefined, + false, + ); + expect(listHeaderKeys(headers)).toEqual([ + "connect-protocol-version", + "content-type", + ]); + }); }); describe("requestHeaderWithCompression", () => { @@ -72,6 +92,7 @@ describe("requestHeaderWithCompression", () => { undefined, [compressionMock], compressionMock, + true, ); expect(listHeaderKeys(headers)).toEqual([ "accept-encoding", @@ -92,6 +113,7 @@ describe("requestHeaderWithCompression", () => { undefined, [compressionMock], compressionMock, + true, ); expect(listHeaderKeys(headers)).toEqual([ "connect-accept-encoding", diff --git a/packages/connect/src/protocol-connect/request-header.ts b/packages/connect/src/protocol-connect/request-header.ts index 50279b7a3..bf84b8fe1 100644 --- a/packages/connect/src/protocol-connect/request-header.ts +++ b/packages/connect/src/protocol-connect/request-header.ts @@ -42,6 +42,7 @@ export function requestHeader( useBinaryFormat: boolean, timeoutMs: number | undefined, userProvidedHeaders: HeadersInit | undefined, + setUserAgent: boolean, ): Headers { const result = new Headers(userProvidedHeaders ?? {}); if (timeoutMs !== undefined) { @@ -58,7 +59,9 @@ export function requestHeader( : contentTypeStreamJson, ); result.set(headerProtocolVersion, protocolVersion); - result.set(headerUserAgent, "CONNECT_ES_USER_AGENT"); + if (setUserAgent) { + result.set(headerUserAgent, "CONNECT_ES_USER_AGENT"); + } return result; } @@ -79,12 +82,14 @@ export function requestHeaderWithCompression( userProvidedHeaders: HeadersInit | undefined, acceptCompression: Compression[], sendCompression: Compression | null, + setUserAgent: boolean, ): Headers { const result = requestHeader( methodKind, useBinaryFormat, timeoutMs, userProvidedHeaders, + setUserAgent, ); if (sendCompression != null) { const name = diff --git a/packages/connect/src/protocol-connect/transport.ts b/packages/connect/src/protocol-connect/transport.ts index aac92d4e5..3cf9d141c 100644 --- a/packages/connect/src/protocol-connect/transport.ts +++ b/packages/connect/src/protocol-connect/transport.ts @@ -102,6 +102,7 @@ export function createTransport(opt: CommonTransportOptions): Transport { header, opt.acceptCompression, opt.sendCompression, + true, ), contextValues: contextValues ?? createContextValues(), message, @@ -230,6 +231,7 @@ export function createTransport(opt: CommonTransportOptions): Transport { header, opt.acceptCompression, opt.sendCompression, + true, ), contextValues: contextValues ?? createContextValues(), message: input, diff --git a/packages/connect/src/protocol-grpc-web/handler-factory.spec.ts b/packages/connect/src/protocol-grpc-web/handler-factory.spec.ts index 47ff8d661..f95b9915a 100644 --- a/packages/connect/src/protocol-grpc-web/handler-factory.spec.ts +++ b/packages/connect/src/protocol-grpc-web/handler-factory.spec.ts @@ -182,7 +182,7 @@ describe("createHandlerFactory()", function () { httpVersion: "2.0", method: "POST", url: `https://example.com/${service.typeName}/${method.name}`, - header: requestHeader(true, timeoutMs, undefined), + header: requestHeader(true, timeoutMs, undefined, true), body: createAsyncIterable([encodeEnvelope(0, new Uint8Array(0))]), signal: new AbortController().signal, }); @@ -284,7 +284,7 @@ describe("createHandlerFactory()", function () { httpVersion: "2.0", method: "POST", url: `https://example.com/${service.typeName}/${method.name}`, - header: requestHeader(true, undefined, undefined), + header: requestHeader(true, undefined, undefined, true), body: createAsyncIterable([encodeEnvelope(0, new Uint8Array(0))]), signal: ac.signal, }); diff --git a/packages/connect/src/protocol-grpc-web/request-header.spec.ts b/packages/connect/src/protocol-grpc-web/request-header.spec.ts index c4bc2febb..7ad60ea08 100644 --- a/packages/connect/src/protocol-grpc-web/request-header.spec.ts +++ b/packages/connect/src/protocol-grpc-web/request-header.spec.ts @@ -30,7 +30,7 @@ function listHeaderKeys(header: Headers): string[] { describe("requestHeader", () => { it("should create request headers", () => { - const headers = requestHeader(true, undefined, undefined); + const headers = requestHeader(true, undefined, undefined, true); expect(listHeaderKeys(headers)).toEqual([ "content-type", "user-agent", @@ -44,7 +44,7 @@ describe("requestHeader", () => { }); it("should create request headers with timeout", () => { - const headers = requestHeader(true, 10, undefined); + const headers = requestHeader(true, 10, undefined, true); expect(listHeaderKeys(headers)).toEqual([ "content-type", "grpc-timeout", @@ -55,6 +55,17 @@ describe("requestHeader", () => { expect(headers.get("Grpc-Timeout")).toBe("10m"); }); + it("should not set user-agent header", () => { + const headers = requestHeader(true, 10, undefined, false); + expect(listHeaderKeys(headers)).toEqual([ + "content-type", + "grpc-timeout", + "x-grpc-web", + "x-user-agent", + ]); + expect(headers.get("Grpc-Timeout")).toBe("10m"); + }); + it("should create request headers with compression", () => { const compressionMock: Compression = { name: "gzip", @@ -68,6 +79,7 @@ describe("requestHeader", () => { undefined, [compressionMock], compressionMock, + true, ); expect(listHeaderKeys(headers)).toEqual([ "content-type", diff --git a/packages/connect/src/protocol-grpc-web/request-header.ts b/packages/connect/src/protocol-grpc-web/request-header.ts index 3f06939de..d45caee53 100644 --- a/packages/connect/src/protocol-grpc-web/request-header.ts +++ b/packages/connect/src/protocol-grpc-web/request-header.ts @@ -33,6 +33,7 @@ export function requestHeader( useBinaryFormat: boolean, timeoutMs: number | undefined, userProvidedHeaders: HeadersInit | undefined, + setUserAgent: boolean, ): Headers { const result = new Headers(userProvidedHeaders ?? {}); // Note that we do not support the grpc-web-text format. @@ -46,7 +47,9 @@ export function requestHeader( // We use "connect-es/1.2.3" where gRPC would use "grpc-es/1.2.3". // See https://github.com/grpc/grpc/blob/c462bb8d485fc1434ecfae438823ca8d14cf3154/doc/PROTOCOL-HTTP2.md#user-agents result.set(headerXUserAgent, "CONNECT_ES_USER_AGENT"); - result.set(headerUserAgent, "CONNECT_ES_USER_AGENT"); + if (setUserAgent) { + result.set(headerUserAgent, "CONNECT_ES_USER_AGENT"); + } if (timeoutMs !== undefined) { result.set(headerTimeout, `${timeoutMs}m`); } @@ -64,8 +67,14 @@ export function requestHeaderWithCompression( userProvidedHeaders: HeadersInit | undefined, acceptCompression: Compression[], sendCompression: Compression | null, + setUserAgent: boolean, ): Headers { - const result = requestHeader(useBinaryFormat, timeoutMs, userProvidedHeaders); + const result = requestHeader( + useBinaryFormat, + timeoutMs, + userProvidedHeaders, + setUserAgent, + ); if (sendCompression != null) { result.set(headerEncoding, sendCompression.name); } diff --git a/packages/connect/src/protocol-grpc-web/transport.ts b/packages/connect/src/protocol-grpc-web/transport.ts index 21063e5db..50815ed68 100644 --- a/packages/connect/src/protocol-grpc-web/transport.ts +++ b/packages/connect/src/protocol-grpc-web/transport.ts @@ -95,6 +95,7 @@ export function createTransport(opt: CommonTransportOptions): Transport { header, opt.acceptCompression, opt.sendCompression, + true, ), contextValues: contextValues ?? createContextValues(), message, @@ -230,6 +231,7 @@ export function createTransport(opt: CommonTransportOptions): Transport { header, opt.acceptCompression, opt.sendCompression, + true, ), contextValues: contextValues ?? createContextValues(), message: input,