fix(git-sync): forward gzip encoding (#4) + catch read-advertise reject after hijack (#5); fix Export 500 on pages with inline comments
- Review #4: forward HTTP_CONTENT_ENCODING to git http-backend so it inflates gzip'd RPC bodies — a non-trivial `git pull` no longer fails with `fatal: expected 'packfile'`. (git-http-backend + git-http.service) - Review #5: the read-advertisement branch runs under the space lock AFTER reply.hijack(); a reject there (e.g. Redis down) previously left the socket open forever, hanging every clone/fetch. Mirror the push branch: catch, 500 if unwritten, always end the socket. (git-http.service) - GS-EXPORT-500 (QA): a page with an inline comment mark returned HTTP 500 on Export/copy-as-markdown. The Comment mark's renderHTML took the imperative document.createElement branch server-side (the DOM shim used by generateHTML defines window/document), returning a live node with no content hole that crashed prosemirror-model's DOMSerializer under happy-dom. Gate the imperative branch on a real browser (navigator.userAgent contains 'Mozilla'); the server now uses the static DOMOutputSpec form. Verified: export 200 (was 500). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -86,6 +86,11 @@ export interface GitHttpBackendRequest {
|
||||
contentType: string;
|
||||
/** The Git-Protocol request header value, or undefined when absent. */
|
||||
gitProtocol?: string;
|
||||
/** Content-Encoding request header (e.g. `gzip`), or undefined when absent.
|
||||
* git gzips RPC bodies >1KiB; http-backend only inflates when HTTP_CONTENT_ENCODING
|
||||
* is present, so it MUST be forwarded or a non-trivial `git pull` fails with
|
||||
* `fatal: expected 'packfile'` (review #4). */
|
||||
contentEncoding?: string;
|
||||
/** Authenticated user email — used as REMOTE_USER (reflog identity). */
|
||||
remoteUser: string;
|
||||
}
|
||||
@@ -131,6 +136,14 @@ export function buildGitBackendCgiEnv(
|
||||
if (parsed.gitProtocol) {
|
||||
cgiEnv.GIT_PROTOCOL = parsed.gitProtocol;
|
||||
}
|
||||
// HTTP_CONTENT_ENCODING must be forwarded so git http-backend inflates a
|
||||
// gzip'd RPC body (git compresses receive-pack/upload-pack bodies >1KiB).
|
||||
// Without it a non-trivial `git pull` negotiation fails deterministically with
|
||||
// `fatal: expected 'packfile'` (review #4). The body is piped to stdin as-is
|
||||
// (no upstream decompression), so the CGI must do the inflate.
|
||||
if (parsed.contentEncoding) {
|
||||
cgiEnv.HTTP_CONTENT_ENCODING = parsed.contentEncoding;
|
||||
}
|
||||
return cgiEnv;
|
||||
}
|
||||
|
||||
|
||||
@@ -345,6 +345,9 @@ export class GitHttpService implements OnModuleDestroy {
|
||||
queryString: this.extractQueryString(req.url),
|
||||
contentType: this.headerValue(req.headers['content-type']) ?? '',
|
||||
gitProtocol: this.headerValue(req.headers['git-protocol']),
|
||||
// Forward Content-Encoding so http-backend inflates gzip'd RPC bodies
|
||||
// (review #4) — else a non-trivial `git pull` fails with expected 'packfile'.
|
||||
contentEncoding: this.headerValue(req.headers['content-encoding']),
|
||||
remoteUser: user.email,
|
||||
};
|
||||
|
||||
@@ -389,9 +392,30 @@ export class GitHttpService implements OnModuleDestroy {
|
||||
service === 'git-upload-pack') ||
|
||||
parsedPath.subpath === 'HEAD');
|
||||
if (isReadAdvertise) {
|
||||
await this.orchestrator.serveReadAdvertisement(spaceId, () =>
|
||||
this.backend.run(backendRequest, rawReq, rawRes),
|
||||
);
|
||||
// The read-advertise path runs under the space lock (to pin HEAD=main).
|
||||
// AFTER reply.hijack() Fastify no longer manages this response, so a
|
||||
// rejection here (e.g. SpaceLockService.acquire when Redis is down) would
|
||||
// otherwise leave the socket open forever — every clone/fetch hangs until
|
||||
// the client times out (review #5). Mirror the push branch: catch, answer
|
||||
// 500 if nothing was written yet, and always end the raw socket.
|
||||
try {
|
||||
await this.orchestrator.serveReadAdvertisement(spaceId, () =>
|
||||
this.backend.run(backendRequest, rawReq, rawRes),
|
||||
);
|
||||
} catch (err) {
|
||||
this.logger.error(
|
||||
`git-sync: read advertisement failed for space ${spaceId}: ${
|
||||
err instanceof Error ? err.message : String(err)
|
||||
}`,
|
||||
);
|
||||
if (!rawRes.headersSent) {
|
||||
rawRes.statusCode = 500;
|
||||
rawRes.setHeader('Content-Type', 'text/plain');
|
||||
rawRes.end('Internal server error');
|
||||
} else if (!rawRes.writableEnded) {
|
||||
rawRes.end();
|
||||
}
|
||||
}
|
||||
} else {
|
||||
await this.backend.run(backendRequest, rawReq, rawRes);
|
||||
}
|
||||
|
||||
@@ -172,7 +172,27 @@ export const Comment = Mark.create<ICommentOptions, ICommentStorage>({
|
||||
const commentId = HTMLAttributes?.["data-comment-id"] || null;
|
||||
const resolved = HTMLAttributes?.["data-resolved"] || false;
|
||||
|
||||
if (typeof window === "undefined" || typeof document === "undefined") {
|
||||
// Prefer the static array (DOMOutputSpec) form whenever we are NOT in a real
|
||||
// interactive browser. Guarding only on `document`/`window` is insufficient
|
||||
// on the server: `generateHTML()` runs under a DOM shim (happy-dom/jsdom) that
|
||||
// DEFINES both globals, so the imperative `document.createElement` branch ran
|
||||
// server-side and its live node + addEventListener crashed the shim's
|
||||
// DOMSerializer ("Cannot read properties of undefined (reading 'length')"),
|
||||
// turning every Export / copy-as-markdown of a page with an inline comment
|
||||
// into an HTTP 500 (QA GS-EXPORT-500). A real browser has a non-empty
|
||||
// navigator.userAgent; the SSR shims do not — route the server to the safe
|
||||
// static form while keeping the clickable node in the actual editor.
|
||||
const isInteractiveBrowser =
|
||||
typeof window !== "undefined" &&
|
||||
typeof document !== "undefined" &&
|
||||
typeof navigator !== "undefined" &&
|
||||
typeof navigator.userAgent === "string" &&
|
||||
// Real browsers ALL carry "Mozilla" in the UA string (historical); the
|
||||
// server-side DOM shim used by generateHTML() does not (e.g. "Node.js/22").
|
||||
// This keeps the interactive click node ONLY in a true browser and routes
|
||||
// the server to the crash-free static form.
|
||||
navigator.userAgent.includes("Mozilla");
|
||||
if (!isInteractiveBrowser) {
|
||||
return [
|
||||
"span",
|
||||
mergeAttributes(this.options.HTMLAttributes, HTMLAttributes, {
|
||||
|
||||
Reference in New Issue
Block a user