Skip to content

Commit

Permalink
Don't set User-Agent header in connect-web (#912)
Browse files Browse the repository at this point in the history
  • Loading branch information
@srikrsna-buf
srikrsna-buf authored Nov 7, 2023
1 parent fec06c7 commit 24bcfb2
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 16 deletions.
2 changes: 1 addition & 1 deletion packages/connect-web-bench/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
2 changes: 2 additions & 0 deletions packages/connect-web/src/connect-transport.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ export function createConnectTransport(
useBinaryFormat,
timeoutMs,
header,
false,
),
contextValues: contextValues ?? createContextValues(),
message,
Expand Down Expand Up @@ -323,6 +324,7 @@ export function createConnectTransport(
useBinaryFormat,
timeoutMs,
header,
false,
),
contextValues: contextValues ?? createContextValues(),
message: input,
Expand Down
4 changes: 2 additions & 2 deletions packages/connect-web/src/grpc-web-transport.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down
8 changes: 4 additions & 4 deletions packages/connect/src/protocol-connect/handler-factory.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down Expand Up @@ -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,
});
Expand Down Expand Up @@ -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,
});
Expand Down Expand Up @@ -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,
});
Expand Down
26 changes: 24 additions & 2 deletions packages/connect/src/protocol-connect/request-header.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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", () => {
Expand All @@ -72,6 +92,7 @@ describe("requestHeaderWithCompression", () => {
undefined,
[compressionMock],
compressionMock,
true,
);
expect(listHeaderKeys(headers)).toEqual([
"accept-encoding",
Expand All @@ -92,6 +113,7 @@ describe("requestHeaderWithCompression", () => {
undefined,
[compressionMock],
compressionMock,
true,
);
expect(listHeaderKeys(headers)).toEqual([
"connect-accept-encoding",
Expand Down
7 changes: 6 additions & 1 deletion packages/connect/src/protocol-connect/request-header.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}

Expand All @@ -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 =
Expand Down
2 changes: 2 additions & 0 deletions packages/connect/src/protocol-connect/transport.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ export function createTransport(opt: CommonTransportOptions): Transport {
header,
opt.acceptCompression,
opt.sendCompression,
true,
),
contextValues: contextValues ?? createContextValues(),
message,
Expand Down Expand Up @@ -230,6 +231,7 @@ export function createTransport(opt: CommonTransportOptions): Transport {
header,
opt.acceptCompression,
opt.sendCompression,
true,
),
contextValues: contextValues ?? createContextValues(),
message: input,
Expand Down
4 changes: 2 additions & 2 deletions packages/connect/src/protocol-grpc-web/handler-factory.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down Expand Up @@ -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,
});
Expand Down
16 changes: 14 additions & 2 deletions packages/connect/src/protocol-grpc-web/request-header.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -68,6 +79,7 @@ describe("requestHeader", () => {
undefined,
[compressionMock],
compressionMock,
true,
);
expect(listHeaderKeys(headers)).toEqual([
"content-type",
Expand Down
13 changes: 11 additions & 2 deletions packages/connect/src/protocol-grpc-web/request-header.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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`);
}
Expand All @@ -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);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/connect/src/protocol-grpc-web/transport.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export function createTransport(opt: CommonTransportOptions): Transport {
header,
opt.acceptCompression,
opt.sendCompression,
true,
),
contextValues: contextValues ?? createContextValues(),
message,
Expand Down Expand Up @@ -230,6 +231,7 @@ export function createTransport(opt: CommonTransportOptions): Transport {
header,
opt.acceptCompression,
opt.sendCompression,
true,
),
contextValues: contextValues ?? createContextValues(),
message: input,
Expand Down

0 comments on commit 24bcfb2

Please sign in to comment.