Fix memory allocation in PSOperator and Type1FontProgram#703
Open
MaximPlusov wants to merge 1 commit into
Open
Fix memory allocation in PSOperator and Type1FontProgram#703MaximPlusov wants to merge 1 commit into
MaximPlusov wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR adds three resource-consumption safety limits to PostScript processing: ChangesPostScript Resource Safety Limits
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/verapdf/parser/postscript/PSOperator.java`:
- Around line 588-590: In PSOperator (the block checking increment/initial/limit
where it currently does "if (increment == 0 || (increment > 0 && initial >
limit) || (increment < 0 && initial < limit)) { throw new
PostScriptException(...); }"), change the behavior so only a zero increment
throws PostScriptException; remove the throw for direction-mismatch cases and
instead treat them as no-op loops by returning zero iterations (or setting the
loop count/iterator to 0) when (increment > 0 && initial > limit) or (increment
< 0 && initial < limit); keep the check and throw for increment == 0, and ensure
you return the appropriate zero-iteration value from the surrounding method so
callers handle the no-op correctly.
- Around line 540-543: The code narrows the value from
getTopNumber().getInteger() to an int (arraySize) before checking range,
allowing very large integers to wrap; change the validation to check the
BigInteger/long magnitude against MAX_PS_ARRAY_SIZE and non-negativity before
converting to int (i.e., obtain the BigInteger/Number from
getTopNumber().getInteger(), compare it to BigInteger.valueOf(MAX_PS_ARRAY_SIZE)
and zero, and only then call intValueExact()/intValue() to assign to arraySize
or throw PostScriptException if out of range), updating any surrounding logic in
the method in PSOperator where arraySize is used.
In `@src/main/java/org/verapdf/pd/font/type1/Type1FontProgram.java`:
- Around line 195-196: In Type1FontProgram change the recursion guard so it
throws when depth is greater than or equal to the limit: replace the current
condition using depth and MAX_TO_EXECUTE_DEPTH (currently "depth >
MAX_TO_EXECUTE_DEPTH") with a check that fails on equality as well (e.g., "depth
>= MAX_TO_EXECUTE_DEPTH") so the PostScriptException in the toExecute recursion
path is raised at the intended maximum; update the clause that throws
PostScriptException("Type 1 font program exceeded toExecute recursion depth")
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e51e25a-5fff-46b0-a002-e29c819ca54f
📒 Files selected for processing (2)
src/main/java/org/verapdf/parser/postscript/PSOperator.javasrc/main/java/org/verapdf/pd/font/type1/Type1FontProgram.java
Comment on lines
540
to
+543
| int arraySize = getTopNumber().getInteger().intValue(); | ||
| if (arraySize < 0 || arraySize > MAX_PS_ARRAY_SIZE) { | ||
| throw new PostScriptException("Array size " + arraySize + " is out of range"); | ||
| } |
There was a problem hiding this comment.
Validate array size before narrowing to int.
Line 540 narrows to int before range validation, so oversized integers can wrap and evade the intended guard.
Proposed fix
- int arraySize = getTopNumber().getInteger().intValue();
- if (arraySize < 0 || arraySize > MAX_PS_ARRAY_SIZE) {
- throw new PostScriptException("Array size " + arraySize + " is out of range");
+ long requestedSize = getTopNumber().getInteger();
+ if (requestedSize < 0 || requestedSize > MAX_PS_ARRAY_SIZE) {
+ throw new PostScriptException("Array size " + requestedSize + " is out of range");
}
+ int arraySize = Math.toIntExact(requestedSize);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| int arraySize = getTopNumber().getInteger().intValue(); | |
| if (arraySize < 0 || arraySize > MAX_PS_ARRAY_SIZE) { | |
| throw new PostScriptException("Array size " + arraySize + " is out of range"); | |
| } | |
| long requestedSize = getTopNumber().getInteger(); | |
| if (requestedSize < 0 || requestedSize > MAX_PS_ARRAY_SIZE) { | |
| throw new PostScriptException("Array size " + requestedSize + " is out of range"); | |
| } | |
| int arraySize = Math.toIntExact(requestedSize); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/org/verapdf/parser/postscript/PSOperator.java` around lines 540
- 543, The code narrows the value from getTopNumber().getInteger() to an int
(arraySize) before checking range, allowing very large integers to wrap; change
the validation to check the BigInteger/long magnitude against MAX_PS_ARRAY_SIZE
and non-negativity before converting to int (i.e., obtain the BigInteger/Number
from getTopNumber().getInteger(), compare it to
BigInteger.valueOf(MAX_PS_ARRAY_SIZE) and zero, and only then call
intValueExact()/intValue() to assign to arraySize or throw PostScriptException
if out of range), updating any surrounding logic in the method in PSOperator
where arraySize is used.
Comment on lines
+588
to
+590
| if (increment == 0 || (increment > 0 && initial > limit) || (increment < 0 && initial < limit)) { | ||
| throw new PostScriptException("Wrong increment value " + increment); | ||
| } |
There was a problem hiding this comment.
Don’t throw on non-entering for loops; return zero iterations instead.
The direction-mismatch cases should be valid no-op loops, not PostScriptException. Throwing here breaks expected for semantics.
Proposed fix
- if (increment == 0 || (increment > 0 && initial > limit) || (increment < 0 && initial < limit)) {
+ if (increment == 0) {
throw new PostScriptException("Wrong increment value " + increment);
}
+ if ((increment > 0 && initial > limit) || (increment < 0 && initial < limit)) {
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (increment == 0 || (increment > 0 && initial > limit) || (increment < 0 && initial < limit)) { | |
| throw new PostScriptException("Wrong increment value " + increment); | |
| } | |
| if (increment == 0) { | |
| throw new PostScriptException("Wrong increment value " + increment); | |
| } | |
| if ((increment > 0 && initial > limit) || (increment < 0 && initial < limit)) { | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/org/verapdf/parser/postscript/PSOperator.java` around lines 588
- 590, In PSOperator (the block checking increment/initial/limit where it
currently does "if (increment == 0 || (increment > 0 && initial > limit) ||
(increment < 0 && initial < limit)) { throw new PostScriptException(...); }"),
change the behavior so only a zero increment throws PostScriptException; remove
the throw for direction-mismatch cases and instead treat them as no-op loops by
returning zero iterations (or setting the loop count/iterator to 0) when
(increment > 0 && initial > limit) or (increment < 0 && initial < limit); keep
the check and throw for increment == 0, and ensure you return the appropriate
zero-iteration value from the surrounding method so callers handle the no-op
correctly.
Comment on lines
+195
to
+196
| if (depth > MAX_TO_EXECUTE_DEPTH) { | ||
| throw new PostScriptException("Type 1 font program exceeded toExecute recursion depth"); |
There was a problem hiding this comment.
Fix off-by-one in recursion depth guard.
depth > MAX_TO_EXECUTE_DEPTH permits one extra recursive level beyond the stated max. If the cap is intended to be 64, this should fail on equality too.
Suggested patch
- if (depth > MAX_TO_EXECUTE_DEPTH) {
+ if (depth >= MAX_TO_EXECUTE_DEPTH) {
throw new PostScriptException("Type 1 font program exceeded toExecute recursion depth");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (depth > MAX_TO_EXECUTE_DEPTH) { | |
| throw new PostScriptException("Type 1 font program exceeded toExecute recursion depth"); | |
| if (depth >= MAX_TO_EXECUTE_DEPTH) { | |
| throw new PostScriptException("Type 1 font program exceeded toExecute recursion depth"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/org/verapdf/pd/font/type1/Type1FontProgram.java` around lines
195 - 196, In Type1FontProgram change the recursion guard so it throws when
depth is greater than or equal to the limit: replace the current condition using
depth and MAX_TO_EXECUTE_DEPTH (currently "depth > MAX_TO_EXECUTE_DEPTH") with a
check that fails on equality as well (e.g., "depth >= MAX_TO_EXECUTE_DEPTH") so
the PostScriptException in the toExecute recursion path is raised at the
intended maximum; update the clause that throws PostScriptException("Type 1 font
program exceeded toExecute recursion depth") accordingly.
bdoubrov
approved these changes
May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit