Fix integer overflows when dealing with templates. #21628
Conversation
The overflows were happening in two places, the parsing of the template that implicitly truncates the `order` when its value does not fall into the `integer` range, and the comparator that sorts templates in ascending order, since it returns `order2-order1`, which might overflow. Closes elastic#21622
nik9000
left a comment
nik9000
left a comment
There was a problem hiding this comment.
It'd be nice to have yaml test with a template with a big negative value just to confirm 100% that the fix flows all the way up properly.
| public static long toLongExact(Number n) { | ||
| if (n instanceof Byte || n instanceof Short || n instanceof Integer | ||
| || n instanceof Long || n instanceof Float || n instanceof Double) { | ||
| double d = n.doubleValue(); |
There was a problem hiding this comment.
I think it'd be a bit cleaner not to do this for Byte, Short, Integer, and Long.
I wanted to avoid going that route since I don't think we want to have an overflow REST test for every numeric parameter we have in our APIs. So I focused on low-level tests and make sure that this fixed the reported issue manually. |
We don't, but since this overflow came up in this context I think it is worth testing it in this context. Others aren't needed. |
|
Sorry but I feel quite strongly against writing a REST test to check for overflows. I'll go with the majority if more people think there should be such a test, but on my end I would like to avoid having such a test. |
Just merge it then. |
The overflows were happening in two places, the parsing of the template that implicitly truncates the `order` when its value does not fall into the `integer` range, and the comparator that sorts templates in ascending order, since it returns `order2-order1`, which might overflow. Closes #21622
The overflows were happening in two places, the parsing of the template that implicitly truncates the `order` when its value does not fall into the `integer` range, and the comparator that sorts templates in ascending order, since it returns `order2-order1`, which might overflow. Closes #21622
* master: (42 commits) Add support for merging custom meta data in tribe node (elastic#21552) [DOCS] Show EC2's auto attribute (elastic#21474) Add information about the removal of store throttling to the migration guide. Add a recommendation against large documents to the docs. (elastic#21652) Add indices options tests to search api REST tests (elastic#21701) Fixing indentation in geospatial querying example. (elastic#21682) Fix typo in filters aggregation docs (elastic#21690) Add BWC layer for Exceptions (elastic#21694) Add checkstyle rule to forbid empty javadoc comments (elastic#20881) Docs: Added offline install link for discovery-file plugin remove pointless catch exception in TransportSearchAction (elastic#21689) Rename ClusterState#lookupPrototypeSafe to `lookupPrototype` and remove previous "unsafe" unused variant (elastic#21686) Use a buffer to do character to byte conversion in StreamOutput#writeString (elastic#21680) Fix integer overflows when dealing with templates. (elastic#21628) Fix highlighting on a stored keyword field (elastic#21645) Set execute permissions for native plugin programs (elastic#21657) adjust visibility of DiscoveryNodes.Delta constructor Remove unused DiscoveryNodes.Delta constructor Remove unused DiscoveryNode#removeDeadMembers public method Remove minNodeVersion and corresponding public `getSmallestVersion` getter method from DiscoveryNodes ...
clintongormley
added
:Data Management/Indices APIs
3 participants
The overflows were happening in two places, the parsing of the template that
implicitly truncates the
orderwhen its value does not fall into theintegerrange, and the comparator that sorts templates in ascending order, since it
returns
order2-order1, which might overflow.Closes #21622