Skip to content

Conversation

@polyfractal
Copy link
Contributor

Due to the fallthrough logic, DateIntervalWrapper assumed that an otherwise unparsable interval was a legacy fixed millis interval. This could then NPE if the interval was just illegal ("foobar").

This commit correctly checks if the legacy millis parsing fails too, and throws an IllegalArgumentException at that point signaling the provided interval is bad.

Note: Only targeting master since the 7.x backport is still WIP. I'll incorporate this fix directly into the backport.

Closes #41970

Due to the fallthrough logic, DateIntervalWrapper assumed that an
otherwise unparsable interval was a legacy fixed millis interval. This
could then NPE if the interval was just illegal ("foobar").

This commit correctly checks if the legacy millis parsing fails too,
and throws an IllegalArgumentException at that point signaling the
provided interval is bad.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@polyfractal
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/docbldesx

@polyfractal polyfractal merged commit 2e26a5d into elastic:master May 9, 2019
@polyfractal
Copy link
Contributor Author

Thanks @jimczi :)

gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
Due to the fallthrough logic, DateIntervalWrapper assumed that an
otherwise unparsable interval was a legacy fixed millis interval. This
could then NPE if the interval was just illegal ("foobar").

This commit correctly checks if the legacy millis parsing fails too,
and throws an IllegalArgumentException at that point signaling the
provided interval is bad.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DateIntervalWrapper can NPE with nonsensical intervals

4 participants