-
Notifications
You must be signed in to change notification settings - Fork 6
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
[TEX-537] Refactor compression #555
Changes from all commits
c2e7f52
6474958
2bcf63d
eb337d7
8d91ff3
a9efac5
1ca870c
4ed1aa3
26e0391
fa3152b
c1a3a68
17fc13c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import zlib from "zlib"; | ||
|
||
export type CompressionAlgorithm = "br" | "gzip" | "none"; | ||
|
||
export function compressBuffer( | ||
algorithm: CompressionAlgorithm, | ||
buffer: Buffer, | ||
): Buffer { | ||
switch (algorithm) { | ||
case "none": | ||
return buffer; | ||
case "br": | ||
return zlib.brotliCompressSync(buffer); | ||
case "gzip": | ||
return zlib.gzipSync(buffer); | ||
default: | ||
throw new Error(`Unhandled compression algorithm value "${algorithm}"`); | ||
} | ||
} | ||
|
||
export function decompressBuffer( | ||
algorithm: CompressionAlgorithm, | ||
buffer: Buffer, | ||
): Buffer { | ||
switch (algorithm) { | ||
case "none": | ||
return buffer; | ||
case "br": | ||
return zlib.brotliDecompressSync(buffer); | ||
case "gzip": | ||
return zlib.gunzipSync(buffer); | ||
default: | ||
throw new Error(`Unhandled compression algorithm value "${algorithm}"`); | ||
} | ||
} | ||
|
||
export function convertHttpContentEncodingToCompressionAlgorithm( | ||
contentEncoding: string, | ||
): CompressionAlgorithm { | ||
switch (contentEncoding) { | ||
case "": | ||
return "none"; | ||
case "br": | ||
return "br"; | ||
case "gzip": | ||
return "gzip"; | ||
default: | ||
throw new Error(`Unhandled content-encoding value "${contentEncoding}"`); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
import brotli from "brotli"; | ||
import zlib from "zlib"; | ||
import { ParsedMediaType as ParsedContentType } from "content-type"; | ||
import { | ||
convertHttpContentEncodingToCompressionAlgorithm, | ||
decompressBuffer, | ||
} from "./compression"; | ||
|
||
/** | ||
* Headers of a request or response. | ||
|
@@ -35,7 +37,7 @@ export interface HttpResponse { | |
body: Buffer; | ||
} | ||
|
||
export function getHeaderAsString( | ||
export function getHttpHeaderAsString( | ||
headers: HttpHeaders, | ||
headerName: string, | ||
): string { | ||
|
@@ -49,35 +51,28 @@ export function getHeaderAsString( | |
} | ||
} | ||
|
||
export function getHttpRequestContentType(request: HttpRequest): string { | ||
export function getHttpContentEncoding(r: HttpRequest | HttpResponse): string { | ||
return getHttpHeaderAsString(r.headers, "content-encoding"); | ||
} | ||
|
||
export function getHttpContentType(r: HttpRequest | HttpResponse): string { | ||
return ( | ||
getHeaderAsString(request.headers, "content-type") || | ||
getHttpHeaderAsString(r.headers, "content-type") || | ||
"application/octet-stream" | ||
); | ||
} | ||
|
||
export function getHttpRequestBodyDecoded(request: HttpRequest): Buffer { | ||
// Process the content-encoding before looking at the content-type. | ||
const contentEncoding = getHeaderAsString( | ||
request.headers, | ||
"content-encoding", | ||
); | ||
switch (contentEncoding) { | ||
case "": | ||
return request.body; | ||
case "br": | ||
return Buffer.from(brotli.decompress(request.body)); | ||
case "gzip": | ||
return zlib.gunzipSync(request.body); | ||
default: | ||
throw Error(`Unhandled content-encoding value "${contentEncoding}"`); | ||
Comment on lines
-65
to
-73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved this common logic into |
||
} | ||
export function getHttpBodyDecoded(r: HttpRequest | HttpResponse): Buffer { | ||
const contentEncoding = getHttpHeaderAsString(r.headers, "content-encoding"); | ||
const compressionAlgorithm = | ||
convertHttpContentEncodingToCompressionAlgorithm(contentEncoding); | ||
return decompressBuffer(compressionAlgorithm, r.body); | ||
} | ||
|
||
export function decodeHttpRequestBodyToString( | ||
request: HttpRequest, | ||
export function decodeHttpBodyToString( | ||
r: HttpRequest | HttpResponse, | ||
contentType: ParsedContentType, | ||
): string { | ||
const encoding = contentType.parameters.charset as BufferEncoding | undefined; | ||
return getHttpRequestBodyDecoded(request).toString(encoding || "utf-8"); | ||
return getHttpBodyDecoded(r).toString(encoding || "utf-8"); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,4 @@ | ||
import brotli from "brotli"; | ||
import { gzipSync } from "zlib"; | ||
import { brotliCompressSync, gzipSync } from "zlib"; | ||
import { persistTape, reviveTape, redactRequestHeaders } from "./persistence"; | ||
|
||
// Note the repetition. This is necessary otherwise Brotli compression | ||
|
@@ -20,13 +19,15 @@ const BINARY_RESPONSE = Buffer.from([ | |
]); | ||
|
||
const UTF8_REQUEST_BROTLI = Buffer.from( | ||
brotli.compress(Buffer.from(UTF8_REQUEST, "utf8"))!, | ||
brotliCompressSync(Buffer.from(UTF8_REQUEST, "utf8"))!, | ||
); | ||
const UTF8_RESPONSE_BROTLI = Buffer.from( | ||
brotli.compress(Buffer.from(UTF8_RESPONSE, "utf8"))!, | ||
brotliCompressSync(Buffer.from(UTF8_RESPONSE, "utf8"))!, | ||
); | ||
const BINARY_REQUEST_BROTLI = Buffer.from(brotliCompressSync(BINARY_REQUEST)!); | ||
const BINARY_RESPONSE_BROTLI = Buffer.from( | ||
brotliCompressSync(BINARY_RESPONSE)!, | ||
); | ||
const BINARY_REQUEST_BROTLI = Buffer.from(brotli.compress(BINARY_REQUEST)!); | ||
const BINARY_RESPONSE_BROTLI = Buffer.from(brotli.compress(BINARY_RESPONSE)!); | ||
|
||
const UTF8_REQUEST_GZIP = gzipSync(Buffer.from(UTF8_REQUEST, "utf8")); | ||
const UTF8_RESPONSE_GZIP = gzipSync(Buffer.from(UTF8_RESPONSE, "utf8")); | ||
|
@@ -221,7 +222,7 @@ describe("Persistence", () => { | |
}, | ||
body: { | ||
encoding: "base64", | ||
data: "GxcAAI6UrMm1WkAERl0HoDFuCn3CIekc", | ||
data: "GxcA+I+UrMm1WkAERl0HoDFuCn3CAZLOAQ==", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The brotli encoding default compression configuration differs between the old library and the new NodeJS builtin implementation. This is why hard-coded expected encoding here differs. |
||
}, | ||
}, | ||
response: { | ||
|
@@ -233,7 +234,7 @@ describe("Persistence", () => { | |
}, | ||
body: { | ||
encoding: "base64", | ||
data: "GxcAAI6UrPmFmgFmOV+HoM3+C33CIe4U", | ||
data: "GxcA+I+UrPmFmgFmOV+HoM3+C33CAeJOAQ==", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The brotli encoding default compression configuration differs between the old library and the new NodeJS builtin implementation. This is why hard-coded expected encoding here differs. |
||
}, | ||
}, | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,17 @@ | ||
import brotli from "brotli"; | ||
import fs from "fs-extra"; | ||
import yaml from "js-yaml"; | ||
import path from "path"; | ||
import { gunzipSync, gzipSync } from "zlib"; | ||
import { HttpHeaders } from "./http"; | ||
import { | ||
CompressionAlgorithm, | ||
PersistedBuffer, | ||
PersistedTapeRecord, | ||
TapeRecord, | ||
} from "./tape"; | ||
compressBuffer, | ||
convertHttpContentEncodingToCompressionAlgorithm, | ||
} from "./compression"; | ||
import { | ||
getHttpBodyDecoded, | ||
getHttpContentEncoding, | ||
HttpRequest, | ||
HttpResponse, | ||
} from "./http"; | ||
import { PersistedBuffer, PersistedTapeRecord, TapeRecord } from "./tape"; | ||
|
||
/** | ||
* Persistence layer to save tapes to disk and read them from disk. | ||
|
@@ -94,12 +96,12 @@ export function persistTape(record: TapeRecord): PersistedTapeRecord { | |
method: record.request.method, | ||
path: record.request.path, | ||
headers: record.request.headers, | ||
body: serialiseBuffer(record.request.body, record.request.headers), | ||
body: serialiseForTape(record.request), | ||
}, | ||
response: { | ||
status: record.response.status, | ||
headers: record.response.headers, | ||
body: serialiseBuffer(record.response.body, record.response.headers), | ||
body: serialiseForTape(record.response), | ||
}, | ||
}; | ||
} | ||
|
@@ -120,22 +122,12 @@ export function reviveTape(persistedRecord: PersistedTapeRecord): TapeRecord { | |
}; | ||
} | ||
|
||
export function serialiseBuffer( | ||
buffer: Buffer, | ||
headers: HttpHeaders, | ||
): PersistedBuffer { | ||
const header = headers["content-encoding"]; | ||
const contentEncoding = typeof header === "string" ? header : undefined; | ||
const originalBuffer = buffer; | ||
let compression: CompressionAlgorithm = "none"; | ||
if (contentEncoding === "br") { | ||
buffer = Buffer.from(brotli.decompress(buffer)); | ||
compression = "br"; | ||
} | ||
if (contentEncoding === "gzip") { | ||
buffer = gunzipSync(buffer); | ||
compression = "gzip"; | ||
} | ||
Comment on lines
-130
to
-138
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved this common logic into |
||
function serialiseForTape(r: HttpRequest | HttpResponse): PersistedBuffer { | ||
const buffer = getHttpBodyDecoded(r); | ||
const contentEncoding = getHttpContentEncoding(r); | ||
const compressionAlgorithm = | ||
convertHttpContentEncodingToCompressionAlgorithm(contentEncoding); | ||
|
||
const utf8Representation = buffer.toString("utf8"); | ||
try { | ||
// Can it be safely stored and recreated in YAML? | ||
|
@@ -148,17 +140,18 @@ export function serialiseBuffer( | |
return { | ||
encoding: "utf8", | ||
data: utf8Representation, | ||
compression, | ||
compression: compressionAlgorithm, | ||
}; | ||
} | ||
} catch { | ||
// Fall through. | ||
} | ||
|
||
// No luck. Fall back to Base64, persisting the original buffer | ||
// since we might as well store it in its compressed state. | ||
return { | ||
encoding: "base64", | ||
data: originalBuffer.toString("base64"), | ||
data: r.body.toString("base64"), | ||
}; | ||
} | ||
|
||
|
@@ -170,21 +163,7 @@ function unserialiseBuffer(persisted: PersistedBuffer): Buffer { | |
break; | ||
case "utf8": | ||
buffer = Buffer.from(persisted.data, "utf8"); | ||
if (persisted.compression === "br") { | ||
// TODO: Find a workaround for the new compressed message not necessarily | ||
// being identical to what was originally sent (update Content-Length?). | ||
const compressed = brotli.compress(buffer); | ||
if (compressed) { | ||
buffer = Buffer.from(compressed); | ||
} else { | ||
throw new Error(`Brotli compression failed!`); | ||
} | ||
} | ||
if (persisted.compression === "gzip") { | ||
// TODO: Find a workaround for the new compressed message not necessarily | ||
// being identical to what was originally sent (update Content-Length?). | ||
buffer = gzipSync(buffer); | ||
} | ||
Comment on lines
-173
to
-187
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved this common logic into |
||
buffer = compressBuffer(persisted.compression || "none", buffer); | ||
break; | ||
default: | ||
throw new Error(`Unsupported encoding!`); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import { CompressionAlgorithm } from "./compression"; | ||
import { HttpHeaders, HttpRequest, HttpResponse } from "./http"; | ||
|
||
/** | ||
|
@@ -37,8 +38,6 @@ export type PersistedBuffer = | |
} | ||
| { | ||
encoding: "utf8"; | ||
compression: CompressionAlgorithm; | ||
compression: CompressionAlgorithm | undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Older tapes don't have the |
||
data: string; | ||
}; | ||
|
||
export type CompressionAlgorithm = "br" | "gzip" | "none"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved into |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,7 @@ export function setupServers({ | |
defaultTapeName, | ||
host: TEST_SERVER_HOST, | ||
timeout: 100, | ||
enableLogging: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated change: enable logging on the proxay instance running in the integration tests. |
||
unframeGrpcWebJsonRequestsHostnames, | ||
}); | ||
await Promise.all([ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replacing this third-party library with the brotli implementation provided by the NodeJS builtin
zlib
module.