lib: strnum - Reject empty string in str_to_float() and str_to_double() - #309
Open
Nishuuzz wants to merge 2 commits into
Open
lib: strnum - Reject empty string in str_to_float() and str_to_double()#309Nishuuzz wants to merge 2 commits into
Nishuuzz wants to merge 2 commits into
Conversation
strtof() and strtod() perform no conversion for an empty string: they
return 0 and leave endp pointing at str. The existing check only looked
at *endp, which is NUL in that case, so str_to_float("") and
str_to_double("") returned 0 and stored 0 in *num_r. Every other
str_to_*() in this file rejects input that isn't a number.
Check that endp moved past str as well.
The only caller in tree is dict-sql's DICT_SQL_TYPE_DOUBLE, which
accepted an empty field value and stored 0.0 for it. The
DICT_SQL_TYPE_UINT case next to it rejects the same input, since
str_to_int64() validates it.
Only the empty string changes behaviour; any other input that strto[fd]()
refuses already leaves a non-NUL character behind for the *endp check to
catch.
Introduced in e8274de.
test_str_to_float() called test_begin("str_to_int"), so a failure in it
was reported under the name of an unrelated test.
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
1 participant
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.
str_to_float("")andstr_to_double("")return 0 and store 0 in*num_rinstead of failing.strtof()/strtod()perform no conversion for an empty string. They return 0 and leaveendppointing atstr. The check in both functions only looks at*endp, and*endpis the terminating NUL, so the empty string passes as a valid 0. Every otherstr_to_*()in strnum.c rejects input that isn't a number, so this one is the odd one out.The fix is to also require that
endpmoved paststr.Where it shows up in tree: dict-sql's
DICT_SQL_TYPE_DOUBLEcase accepts an empty field value and stores 0.0 for it, whileDICT_SQL_TYPE_UINTsitting right above it rejects the same value becausestr_to_int64()validates it. Two adjacent cases in one switch disagreeing about empty input.How I checked it: I couldn't build Dovecot on this machine (Windows, no autotools), so I pulled
str_to_float()/str_to_double()out verbatim and compiled them standalone with gcc, before and after the change. Running every string of length 0 to 3 over the characters0 1 . - + e x space n i— 1111 inputs, covering digits, signs, exponents, hex and inf/nan prefixes and whitespace — the two versions differ on exactly one input, the empty string. Nothing else moves, which matches the reasoning above: anythingstrto[fd]()refuses leaves a non-NUL character behind for the*endpcheck to catch.I also ran the
tests[]table fromtest_str_to_float()against both versions. The seven existing cases pass either way; the new""case fails before the change and passes after.The second commit is a one-liner for
test_str_to_float()callingtest_begin("str_to_int"), which would have reported the new failure under the wrong test name. Happy to drop it if you'd rather keep it separate.Introduced in e8274de.