-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(remote-control): cache rewritten tunnel responses with ETag validation #3718
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| Reuse unchanged Remote Control assets across page loads instead of retransferring them. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import { createHash } from 'node:crypto'; | ||
| import { hostname, platform } from 'node:os'; | ||
| import { join } from 'node:path'; | ||
| import { request as httpRequest, validateHeaderName, validateHeaderValue } from 'node:http'; | ||
|
|
@@ -248,6 +249,26 @@ function isGzipCompressibleType(contentType: string): boolean { | |
| return mime.startsWith('text/') || GZIP_COMPRESSIBLE_TYPES.has(mime); | ||
| } | ||
|
|
||
| function rewrittenResponseETag(body: Buffer): string { | ||
| return `W/"${createHash('sha256').update(body).digest('hex')}"`; | ||
| } | ||
|
|
||
| function requestMatchesETag( | ||
| headers: readonly [string, string][], | ||
| etag: string, | ||
| ): boolean { | ||
| const candidates = [etag, etag.replace(/^W\//, '')]; | ||
| for (const [name, value] of headers) { | ||
| if (name.toLowerCase() !== 'if-none-match') continue; | ||
| for (const token of value.split(',')) { | ||
| const candidate = token.trim(); | ||
| if (candidate === '*') return true; | ||
| if (candidates.includes(candidate)) return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| export async function startRemoteControl( | ||
| options: RemoteControlOptions, | ||
| ): Promise<RemoteControlHandle> { | ||
|
|
@@ -856,7 +877,18 @@ function requestLocalHttp( | |
| : receivedBody; | ||
| const rewritten = body !== receivedBody; | ||
| const headers = filterResponseHeaders(response.rawHeaders, rewritten); | ||
| if (rewritten) headers.push('Cache-Control', 'no-cache'); | ||
| if (rewritten) { | ||
| const etag = rewrittenResponseETag(body); | ||
| headers.push('Cache-Control', 'no-cache', 'ETag', etag); | ||
|
Comment on lines
+881
to
+882
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.
When rewritten HTML, JS, or CSS is large enough for gzip negotiation, this strong ETag is computed from the identity body and then returned unchanged for both gzip and identity responses. Those representations are not byte-identical, so clients and intermediaries cannot safely use the tag for strong cross-variant operations such as range or cache combination; mark the shared validator weak or derive separate tags from the final encoded bodies. Useful? React with 👍 / 👎. |
||
| const statusCode = response.statusCode ?? 502; | ||
| const revalidatable = | ||
| (parsed.method === 'GET' || parsed.method === 'HEAD') && | ||
| statusCode >= 200 && | ||
| statusCode < 300; | ||
| if (revalidatable && requestMatchesETag(parsed.headers, etag)) { | ||
| return Buffer.from(`HTTP/1.1 304 Not Modified\r\n${headerLines(headers)}\r\n\r\n`); | ||
| } | ||
| } | ||
| const negotiated = | ||
| response.headers['content-encoding'] === undefined && | ||
| response.statusCode !== 206 && | ||
|
|
@@ -877,9 +909,6 @@ function requestLocalHttp( | |
| if (negotiated && acceptsGzipEncoding(parsed.headers)) { | ||
| body = await gzipAsync(body); | ||
| headers.push('Content-Encoding', 'gzip'); | ||
| for (let index = headers.length - 2; index >= 0; index -= 2) { | ||
| if (headers[index]!.toLowerCase() === 'etag') headers.splice(index, 2); | ||
| } | ||
| } | ||
| headers.push('Content-Length', String(body.length)); | ||
| const statusCode = response.statusCode ?? 502; | ||
|
|
@@ -898,7 +927,7 @@ function requestLocalHttp( | |
| }); | ||
| } | ||
|
|
||
| function filterResponseHeaders(rawHeaders: readonly string[], blockCacheControl = false): string[] { | ||
| function filterResponseHeaders(rawHeaders: readonly string[], blockCacheValidators = false): string[] { | ||
| const connectionHeaders = new Set<string>(); | ||
| for (let index = 0; index < rawHeaders.length; index += 2) { | ||
| if (rawHeaders[index]!.toLowerCase() === 'connection') { | ||
|
|
@@ -914,7 +943,12 @@ function filterResponseHeaders(rawHeaders: readonly string[], blockCacheControl | |
| if (BLOCKED_RESPONSE_HEADERS.has(lower) || connectionHeaders.has(lower)) { | ||
| continue; | ||
| } | ||
| if (blockCacheControl && lower === 'cache-control') continue; | ||
| if ( | ||
| blockCacheValidators && | ||
| (lower === 'cache-control' || lower === 'etag' || lower === 'last-modified') | ||
| ) { | ||
| continue; | ||
| } | ||
| result.push(name, rawHeaders[index + 1]!); | ||
| } | ||
| return result; | ||
|
|
||
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.
For a
HEADrequest, Node's HTTP client exposes no response body, so this hashes an empty JS/CSS body (or only the injected HTML fragment) rather than the representation returned byGET. Consequently, changed assets keep the same HEAD validator and a later conditional HEAD can incorrectly receive304 Not Modified; the generated validator also disagrees with the GET validator. Either limit this synthesized validation path to GET or derive HEAD validators from the corresponding full representation.Useful? React with 👍 / 👎.