Skip to content

Guard PDRange constructors against invalid ranges#176

Closed
pingpingy1 wants to merge 1 commit into
apache:trunkfrom
pingpingy1:trunk
Closed

Guard PDRange constructors against invalid ranges#176
pingpingy1 wants to merge 1 commit into
apache:trunkfrom
pingpingy1:trunk

Conversation

@pingpingy1
Copy link
Copy Markdown

This PR intends to provide suitable guards against invalid arguments to the PDRange constructors.

The first case is when the COSArray range parameter is given the null value.
Here, the null value is directly saved to the rangeArray variable, and cannot be set to a non-null value using any other public method.
Hence, I argue that this case should be guarded against in the constructor, either by throwing an exception (for example, using the Objects.requireNonNull method) or by setting the range to a default range (as implemented in my commit using 0..1).

The second case is when the index parameter leads to invalid indices for the range's minimum and maximum values.
Again, the provided index's value is directly saved to the startingIndex variable, and cannot by changed via any public method.
Moreover, no public method could lengthen the rangeArray.
The methods setMin and setMax do not check the validity of the indices 2*startingIndex and 2*startingIndex+1, which could throw exceptions down the line.
Thus, I argue that this case also should be handled here, either by throwing an exception (as implemented in my commit using IllegalArgumentException) or by using a default index (such as 0).

@pingpingy1
Copy link
Copy Markdown
Author

Oops, didn't realize I hadn't written tests for the new guards. My bad.

PDRange has three constructors, two of which takes a `COSArray range`
input to describe the range, and one that also takes an index into the
array.
This commit adds additional guards for when
1. the input array is `null`, or
2. the index does not properly point to two elements of the array.
The first case is handled by setting to the default range 0..1.
The second throws an `IllegalArgumentException`.
@THausherr
Copy link
Copy Markdown
Contributor

I'm not sure if this is helpful, because bad parameters may not just come from users, but from bad PDFs (although unlikely). A user constructing PDRange with a null COSArray will just get an NPE and IMHO this is fine. What remains is the case where a PDF has an array that is too short. It's possible that we do already catch this, for example getEncodeForParameter does so. Such PDFs should not fail with an IllegalArgumentException because this is an unchecked exception.

@pingpingy1 pingpingy1 closed this Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants