throw instead of hanging on incomplete compressed input - #77
Open
knQzx wants to merge 1 commit into
Open
Conversation
The inflate loop only stopped once the stream was finished. Inflater.inflate returns 0 when it needs more input, and finished() stays false in that case, so an empty or truncated payload spun forever at full CPU instead of throwing the DataFormatException the method already declares. Reachable from readCompressedAsStatic, readCompressedAsDynamic and readCompressedAsPreprocessed, which take a caller-supplied byte array.
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
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.
SerializationUtil.decompressdrives the inflater with onlyfinished()as the stop condition:inflatereturns 0 when it needs more input, andfinished()stays false in that case, so an empty or truncated payload makes this spin at full CPU forever. it never throws theDataFormatExceptionthe method already declares, so a caller cannot recover eitherit is reachable from three public entry points that take a caller-supplied array:
readCompressedAsStatic,readCompressedAsDynamicandreadCompressedAsPreprocessed. a short read from a socket or a partially written file is enoughworth noting: random bytes do throw, because the zlib header check catches them. the hang needs either an empty array or a stream that starts correctly and is cut short, for example
Arrays.copyOf(writeCompressed(h), 3)changes
bail out with
DataFormatExceptionwhen the inflater made no progress and is waiting for input or a dictionarytesting
two tests, one with an empty array and one with a truncated stream, both wrapped in
assertTimeoutPreemptivelysince a plainassertThrowswould wedge the runner. both fail with a 5 second timeout before the change and pass afterI could not use gradle locally, its toolchain wants a jdk newer than I have, so I compiled the sources with javac and ran the test class through the junit console launcher: 9 tests pass with the change, 7 pass and 2 time out without it. formatting checked against google-java-format 1.34.1, the version spotless pins