Skip to content

Partition.java does not check the validity of input iterators#236

Open
emopers wants to merge 1 commit into
imglib:masterfrom
emopers:hasNextViolationFix
Open

Partition.java does not check the validity of input iterators#236
emopers wants to merge 1 commit into
imglib:masterfrom
emopers:hasNextViolationFix

Conversation

@emopers

@emopers emopers commented Jan 23, 2019

Copy link
Copy Markdown

Partition.java calls j.previous() on java.util.Iterator j and i.next() on java.util.Iterator i
without checking if there are any elements to iterate over. Because the method is public and the iterators are obtained from inputs, they could be invalid (e.g., an empty list) and lead to an exception. This pull request adds a hasNext() and a hasPrevious() check.

This violation is present in other parts of this class as well. If this fix is acceptable to you, we can
submit another pull request with fixes for all other cases.

Fixing hasNext violation in a public class.
@ctrueden

Copy link
Copy Markdown
Member

Looks good to me! What do you think, @tpietzsch?

@ctrueden

Copy link
Copy Markdown
Member

I retract my LGTM; see #259 (comment).

@ctrueden

Copy link
Copy Markdown
Member

In #259 (comment), @tpietzsch wrote:

it should rather check i.nextIndex() < j.previousIndex() or something (I would need to think about the exact condition). Unfortunately, there is no way to check whether i and j are iterators into the same list, which is also an expectation about the input.

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