-
Notifications
You must be signed in to change notification settings - Fork 4.1k
fix(v8): allow non-ErrorInstance objects in Error.prepareStackTrace #27712
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
base: main
Are you sure you want to change the base?
Changes from 2 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,180 @@ | ||
| import { expect, test } from "bun:test"; | ||
| import { bunEnv, bunExe, tempDirWithFiles } from "harness"; | ||
|
|
||
| // Test for https://github.com/oven-sh/bun/issues/27708 | ||
| // | ||
| // Root cause: Error.prepareStackTrace interacts incorrectly with ES5-style | ||
| // Error subclasses when called through Error.captureStackTrace. | ||
| // | ||
| // 1. @babel/core's loadPartialConfig/transformSync replaces Error.prepareStackTrace | ||
| // with a "stackTraceRewriter" that chains to the original prepareStackTrace. | ||
| // 2. @xmldom/xmldom uses ES5-style Error subclassing (Object.create(Error.prototype)) | ||
| // for its ParseError class, without calling super()/Error.call(this). | ||
| // 3. When Error.captureStackTrace is called on this non-ErrorInstance object, | ||
| // bun's default prepareStackTrace (jsFunctionDefaultErrorPrepareStackTrace) was | ||
| // called through the chain and threw TypeError because it required a JSC ErrorInstance. | ||
| // 4. This TypeError prevents xmldom's ParseError from being properly thrown during | ||
| // malformed SVG parsing, causing the SAX parser to enter an error recovery path | ||
| // that leads to an infinite loop in the position() function's linePattern regex. | ||
|
|
||
| const HELPERS_TS = ` | ||
| import { DOMParser } from "@xmldom/xmldom"; | ||
|
|
||
| interface DomNode { | ||
| readonly tagName?: string; | ||
| readonly nodeType: number; | ||
| readonly attributes?: { | ||
| readonly length: number; | ||
| item(index: number): { readonly name: string; readonly value: string } | null; | ||
| }; | ||
| readonly childNodes?: { | ||
| readonly length: number; | ||
| item(index: number): DomNode | null; | ||
| }; | ||
| } | ||
|
|
||
| const FORBIDDEN_ELEMENTS = new Set(["script", "style", "foreignobject"]); | ||
| const EVENT_HANDLER_PATTERN = /^on[a-z]+$/i; | ||
|
|
||
| interface Violation { rule: string; message: string; } | ||
| interface ValidationResult { valid: boolean; violations: Violation[]; } | ||
|
|
||
| const walkNode = (node: DomNode, violations: Violation[]): void => { | ||
| const tagName = node.tagName?.toLowerCase() ?? ""; | ||
| if (FORBIDDEN_ELEMENTS.has(tagName)) { | ||
| violations.push({ rule: "forbidden-element", message: "Forbidden: " + tagName }); | ||
| } | ||
| if (node.attributes) { | ||
| for (let i = 0; i < node.attributes.length; i++) { | ||
| const attr = node.attributes.item(i); | ||
| if (attr && EVENT_HANDLER_PATTERN.test(attr.name)) { | ||
| violations.push({ rule: "event-handler", message: attr.name }); | ||
| } | ||
| } | ||
| } | ||
| if (node.childNodes) { | ||
| for (let i = 0; i < node.childNodes.length; i++) { | ||
| const child = node.childNodes.item(i); | ||
| if (child && child.nodeType === 1) walkNode(child, violations); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| export const validateSvg = (svg: string): ValidationResult => { | ||
| const violations: Violation[] = []; | ||
| const sanitized = svg.replace(/<!DOCTYPE[^[>]*(?:\\\\[[^\\\\]]*\\\\])?\\\\s*>/gi, "").replace(/<!ENTITY[^>]*>/gi, ""); | ||
| if (svg !== sanitized) violations.push({ rule: "xxe-prevention", message: "DOCTYPE/ENTITY" }); | ||
| try { | ||
| // No onError handler — xmldom defaults to console.error output | ||
| const parser = new DOMParser(); | ||
| const doc = parser.parseFromString(sanitized, "image/svg+xml"); | ||
| if (doc.documentElement) walkNode(doc.documentElement, violations); | ||
| } catch { | ||
| violations.push({ rule: "parse-error", message: "Parse failed" }); | ||
| } | ||
| return { valid: violations.length === 0, violations }; | ||
| }; | ||
| `; | ||
|
|
||
| // Generate a test file that exercises xmldom DOMParser with malformed input | ||
| // to produce stderr output via console.error | ||
| function makeParseTestFile(index: number): string { | ||
| return ` | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { validateSvg } from "./helpers"; | ||
|
|
||
| describe("SVG validation batch ${index}", () => { | ||
| test("accepts valid SVG", () => { | ||
| const r = validateSvg('<svg xmlns="http://www.w3.org/2000/svg"><path d="M0 0h24v24H0z"/></svg>'); | ||
| expect(r.valid).toBe(true); | ||
| }); | ||
| test("rejects script element", () => { | ||
| const r = validateSvg('<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>'); | ||
| expect(r.valid).toBe(false); | ||
| }); | ||
| test("handles malformed SVG", () => { | ||
| const r = validateSvg("<not-valid-svg-${index}>"); | ||
| expect(r).toHaveProperty("valid"); | ||
| }); | ||
| test("handles random garbage", () => { | ||
| for (let i = 0; i < 20; i++) { | ||
| const garbage = Array.from({ length: 50 }, () => String.fromCharCode(Math.floor(Math.random() * 128))).join(""); | ||
| const r = validateSvg(garbage); | ||
| expect(typeof r.valid).toBe("boolean"); | ||
| } | ||
| }); | ||
| }); | ||
| `; | ||
| } | ||
|
|
||
| // Generate the async svgr test file | ||
| const SVGR_TEST = ` | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { transform } from "@svgr/core"; | ||
|
|
||
| describe("SVGR transformation", () => { | ||
| test("transforms SVG to React component", async () => { | ||
| const result = await transform( | ||
| '<svg xmlns="http://www.w3.org/2000/svg"><path d="M0 0h24v24H0z"/></svg>', | ||
| { plugins: ["@svgr/plugin-jsx"] }, | ||
| { componentName: "TestIcon" }, | ||
| ); | ||
| expect(result).toContain("TestIcon"); | ||
| expect(result).toContain("export default"); | ||
| }); | ||
| test("handles SVG with viewBox", async () => { | ||
| const result = await transform( | ||
| '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"><circle cx="12" cy="12" r="10"/></svg>', | ||
| { plugins: ["@svgr/plugin-jsx"] }, | ||
| { componentName: "CircleIcon" }, | ||
| ); | ||
| expect(result).toContain("CircleIcon"); | ||
| }); | ||
| }); | ||
| `; | ||
|
|
||
| test("bun test should not hang with 7+ test files combining xmldom stderr and svgr async", async () => { | ||
| const files: Record<string, string> = { | ||
| "package.json": JSON.stringify({ | ||
| name: "stderr-hang-regression", | ||
| private: true, | ||
| devDependencies: { | ||
| "@svgr/core": "^8.1.0", | ||
| "@svgr/plugin-jsx": "^8.1.0", | ||
| "@xmldom/xmldom": "^0.9.8", | ||
| }, | ||
|
Comment on lines
+142
to
+145
Contributor
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. 🧩 Analysis chain🏁 Script executed: cat -n test/regression/issue/027708.test.ts | sed -n '135,155p'Repository: oven-sh/bun Length of output: 852 🏁 Script executed: # Check the complete devDependencies section to understand the context
rg -A 20 "devDependencies:" test/regression/issue/027708.test.tsRepository: oven-sh/bun Length of output: 622 🏁 Script executed: # Check if there are other test fixtures and how they specify dependencies
fd "\.test\.ts" test/regression/issue/ -x basename {} | head -5Repository: oven-sh/bun Length of output: 146 🏁 Script executed: # Look for patterns in other test files with package.json generation
rg -l "devDependencies" test/regression/ --type tsRepository: oven-sh/bun Length of output: 260 🏁 Script executed: # Check how other regression tests specify dependencies
rg -B 3 -A 5 "devDependencies" test/regression/issue/11806.test.ts test/regression/issue/07740.test.tsRepository: oven-sh/bun Length of output: 1158 🏁 Script executed: # Look at the issue this regression test is for - what problem was it solving?
# Check if there are comments in the test explaining the purpose
rg -B 5 "27708\|hang\|xmldom\|svgr" test/regression/issue/027708.test.ts | head -50Repository: oven-sh/bun Length of output: 37 🏁 Script executed: # Check git history or comments about why caret ranges might be used
git log --oneline -n 20 -- test/regression/issue/027708.test.ts 2>/dev/null || echo "No git history available"Repository: oven-sh/bun Length of output: 133 🏁 Script executed: # Check if there are any guidelines about version pinning in test fixtures
rg "version|pin|exact|caret" test/CLAUDE.md CLAUDE.md test/regression/README.md 2>/dev/null | head -20Repository: oven-sh/bun Length of output: 120 🏁 Script executed: # Get more context about the test and the issue it addresses
rg -B 10 "bun test should not hang with 7\+" test/regression/issue/027708.test.tsRepository: oven-sh/bun Length of output: 448 🏁 Script executed: # Check what the test is actually verifying - does it check for a hang/timeout?
rg -A 30 "bun test should not hang" test/regression/issue/027708.test.ts | head -50Repository: oven-sh/bun Length of output: 964 🏁 Script executed: # Look for the GitHub issue number reference or comments about why these specific versions
rg "27708\|issue" test/regression/issue/027708.test.ts | head -20Repository: oven-sh/bun Length of output: 37 🏁 Script executed: # Check if other tests that use exact versions had a specific reason (like reproducing a bug)
rg -B 5 -A 5 "1\.0\.0" test/regression/issue/07740.test.tsRepository: oven-sh/bun Length of output: 465 🏁 Script executed: # Check what the specific issue `#27708` was about - search for any related commits or PRs
git log --all --oneline --grep="27708\|hang.*svgr\|hang.*xmldom" 2>/dev/null | head -10 || echo "No git log available"Repository: oven-sh/bun Length of output: 37 🏁 Script executed: # Examine the complete test to see if it validates against the behavior at these specific versions
rg -A 80 "bun test should not hang with 7\+" test/regression/issue/027708.test.ts | tail -60Repository: oven-sh/bun Length of output: 1327 🏁 Script executed: # Check if there are any version constraints or lock files in the test fixture approach
rg "bun\.lockb|yarn\.lock|package-lock\.json" test/regression/issue/027708.test.tsRepository: oven-sh/bun Length of output: 37 🏁 Script executed: # Check the purpose: does the test verify behavior at THESE specific versions, or general behavior?
rg -B 2 -A 10 "timeout\|hang\|expect" test/regression/issue/027708.test.ts | grep -A 5 "const result"Repository: oven-sh/bun Length of output: 37 🏁 Script executed: # Verify the complete test flow and check if there are assertions about specific package behavior
rg -A 120 "bun test should not hang with 7\+" test/regression/issue/027708.test.ts | tail -50Repository: oven-sh/bun Length of output: 1327 🏁 Script executed: # Search for any documentation about regression test practices regarding dependency versions
find test -name "README*" -o -name "CLAUDE*" | xargs grep -l "regression\|version" 2>/dev/null | head -5Repository: oven-sh/bun Length of output: 221 🏁 Script executed: # Check if the versions in the caret ranges have known breaking changes
web_search
npm registry `@svgr/core` versions after 8.1.0Repository: oven-sh/bun Length of output: 206 🌐 Web query:
💡 Result:
To list all versions from npm locally: npm view `@svgr/core` versions --jsonCitations:
🌐 Web query:
💡 Result: Are there breaking changes in 0.9.8 itself?No. The Breaking changes you’re likely hitting when upgrading to 0.9.8 (from
|
||
| }), | ||
| "tests/helpers.ts": HELPERS_TS, | ||
| "tests/svgr.test.ts": SVGR_TEST, | ||
| }; | ||
|
|
||
| // Create 7 parse test files to exceed the threshold | ||
| for (let i = 1; i <= 7; i++) { | ||
| files[`tests/parse-${String(i).padStart(2, "0")}.test.ts`] = makeParseTestFile(i); | ||
| } | ||
|
|
||
| const dir = tempDirWithFiles("issue-27708", files); | ||
|
|
||
| // Install dependencies | ||
| const install = Bun.spawnSync({ | ||
| cmd: [bunExe(), "install"], | ||
| cwd: dir, | ||
| env: bunEnv, | ||
| stderr: "pipe", | ||
| stdout: "pipe", | ||
| }); | ||
| expect(install.exitCode).toBe(0); | ||
|
|
||
| // Run bun test — before the fix, this would hang indefinitely. | ||
| // Non-hang is enforced by this test's 60s timeout. | ||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "test"], | ||
| cwd: dir, | ||
| env: bunEnv, | ||
| stderr: "ignore", | ||
| stdout: "ignore", | ||
| }); | ||
|
|
||
| const exitCode = await proc.exited; | ||
| expect(exitCode).toBe(0); | ||
| }, 60_000); // 60s test timeout | ||
Uh oh!
There was an error while loading. Please reload this page.