Conversation
…e parser and rendering pipeline
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87eefabca9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
| } | ||
| _ => {} |
There was a problem hiding this comment.
When input uses valid unbraced primitive arguments, such as "\\sqrt".repeat(4_200) + "x", every command reaches parse_group → parse_function recursively, but this preflight ignores sqrt and no braced parse_expression is entered to increment recursion_depth. Consequently, adversarial input can still exhaust the native or WASM stack instead of returning the promised recursion-limit error; the depth guard needs to cover recursive function-argument parsing rather than only braces and \left.
Useful? React with 👍 / 👎.
…adversarial inputs
Deploying ratex with
|
| Latest commit: |
b1822e1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://111b1b1c.ratex.pages.dev |
| Branch Preview URL: | https://fix-deep.ratex.pages.dev |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c21a642a71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let accent_count = chars.len() - split_idx - 1; | ||
| if accent_count > MAX_RECURSION_DEPTH { |
There was a problem hiding this comment.
When a combining-accent token appears inside already nested syntax, this comparison considers only the accent chain itself. For example, 32 brace groups surrounding x plus 32 U+0301 marks pass both the source preflight and this check, yet produce an AST containing 32 group layers followed by 32 accent layers. This violates the parser's 32-level AST contract and exceeds the structure covered by the 512 KiB boundary tests; include the current parser recursion depth when deciding whether the accent chain fits the remaining budget.
Useful? React with 👍 / 👎.
| "left" | "begingroup" => { | ||
| depth += 1; | ||
| if depth > MAX_RECURSION_DEPTH { | ||
| return true; |
There was a problem hiding this comment.
For valid macro-definition-heavy input such as \def\foo{\left(} repeated 32 times followed by x, handle_def consumes each body as raw tokens, so none of those \left commands enters a parser scope. This scanner nevertheless increments for every \left and only decrements for the definition's closing brace, leaving a net depth increase per definition and rejecting the 32nd flat definition as over-nested. Raw macro-definition bodies need to be skipped or their commands must not affect this structural counter.
Useful? React with 👍 / 👎.
| if previous_depth > MAX_RECURSION_DEPTH { | ||
| return Err(MhchemError::msg("Recursion limit exceeded")); | ||
| } | ||
| MHCHEM_RECURSION_DEPTH.with(|depth| depth.set(previous_depth + 1)); |
There was a problem hiding this comment.
When 32 engine invocations are already active, previous_depth equals MAX_RECURSION_DEPTH, but this condition still permits another invocation and sets the active depth to 33. A terminal 33rd submachine call—for example one receiving empty input and returning immediately—therefore succeeds despite the stated policy that level 33 must be rejected; compare with >= before incrementing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a130abcf83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if depth > MAX_RECURSION_DEPTH { | ||
| return Err(ParseError::recursion_limit_exceeded()); |
There was a problem hiding this comment.
When a prooftree is parsed inside existing recursive syntax, this check considers only the branch depth. For example, 31 enclosing brace groups around a prooftree containing an axiom and 31 unary inferences pass both limits, although layout then traverses the 31 group layers before recursively traversing the 32-level ProofBranch. This exceeds the cumulative depth-32 stack-safety contract; combine the current parser depth with the branch depth before accepting the inference.
Useful? React with 👍 / 👎.
| "def" | "gdef" | "edef" | "xdef" => { | ||
| index = skip_macro_definition_body(input, bytes, index); | ||
| } |
There was a problem hiding this comment.
The raw-body exception covers only the TeX \def family, while \newcommand, \renewcommand, and \providecommand are also consumed by the macro expander as token definitions. Consequently, a valid unused definition whose body contains more than 32 nested braces, followed by ordinary content, is rejected during preflight even though the body is never parsed into an AST. Extend the raw-definition handling to these commands, accounting for their name and optional arity arguments.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a30fa1775
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| b'{' => { | ||
| depth += 1; | ||
| if depth > MAX_RECURSION_DEPTH { | ||
| return true; |
There was a problem hiding this comment.
When braces occur inside an argument consumed as a flat string, this increments the structural depth even though parse_string_group never creates nested AST nodes. For example, a \url{...} or the URL argument of \href containing 33 nested balanced braces is rejected here with Recursion limit exceeded, although URL parsing consumes those tokens iteratively into a single Url node and this input was previously accepted. The preflight should skip raw/URL argument contents just as it skips macro-definition bodies.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0a0d7071e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let previous_depth = MHCHEM_RECURSION_DEPTH.with(Cell::get); | ||
| // mhchem inputs fan out through helper machines before texify. Guard the | ||
| // active engine-call depth directly so engine callers fail before building | ||
| // an over-budget AST, while preserving the public parser boundary. | ||
| if previous_depth >= MAX_RECURSION_DEPTH { |
There was a problem hiding this comment.
When an mhchem invocation is macro-expanded inside existing parser nesting, this counter starts from zero and grants the engine a separate 32-level budget. For example, a \def\foo{...} containing roughly 20 nested \ce calls can bypass the source scan because definition bodies are skipped, and invoking \foo inside 20 brace groups can exceed the cumulative depth-32 policy while each counter remains individually below its limit. Seed or otherwise combine this guard with the active parser depth so macro-expanded mhchem cannot stack both budgets.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1822e196e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| b'{' => { | ||
| depth += 1; | ||
| if depth > MAX_RECURSION_DEPTH { | ||
| return true; |
There was a problem hiding this comment.
When braces delimit arguments to user-defined macros, this preflight counts them as structural nesting even though MacroExpander::consume_arg strips the outer braces before substitution. For example, after \def\id#1{#1}, 33 nested calls of the form \id{\id{...x...}} are rejected here, although expansion produces only x and no nested AST at all; similarly, an identity-macro call inside 32 real groups incorrectly makes valid depth-32 input fail. The scan needs to avoid treating macro-call argument delimiters as parser structure or defer this decision until after expansion.
Useful? React with 👍 / 👎.
1 participant
No description provided.