-
Notifications
You must be signed in to change notification settings - Fork 15.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
protoc: fix consistency with parsing very large decimal numbers #10555
Changes from 8 commits
2270d3f
87f24e4
4e54ec2
35dd193
4c69337
7702355
0bc90b1
f82be68
d6acffb
0362a12
7e745c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -288,6 +288,16 @@ bool Parser::ConsumeInteger64(uint64_t max_value, uint64_t* output, | |
} | ||
} | ||
|
||
bool Parser::TryConsumeInteger64(uint64_t max_value, uint64_t* output) { | ||
if (LookingAtType(io::Tokenizer::TYPE_INTEGER) && | ||
io::Tokenizer::ParseInteger(input_->current().text, max_value, | ||
output)) { | ||
input_->Next(); | ||
return true; | ||
} | ||
return false; | ||
} | ||
|
||
bool Parser::ConsumeNumber(double* output, const char* error) { | ||
if (LookingAtType(io::Tokenizer::TYPE_FLOAT)) { | ||
*output = io::Tokenizer::ParseFloat(input_->current().text); | ||
|
@@ -296,13 +306,19 @@ bool Parser::ConsumeNumber(double* output, const char* error) { | |
} else if (LookingAtType(io::Tokenizer::TYPE_INTEGER)) { | ||
// Also accept integers. | ||
uint64_t value = 0; | ||
if (!io::Tokenizer::ParseInteger(input_->current().text, | ||
if (io::Tokenizer::ParseInteger(input_->current().text, | ||
std::numeric_limits<uint64_t>::max(), | ||
&value)) { | ||
*output = value; | ||
} else if (input_->current().text[0] == '0') { | ||
// octal or hexadecimal; don't bother parsing as float | ||
AddError("Integer out of range."); | ||
// We still return true because we did, in fact, parse a number. | ||
} else if (!io::Tokenizer::TryParseFloat(input_->current().text, output)) { | ||
// out of int range, and not valid float? 🤷 | ||
AddError("Integer out of range."); | ||
// We still return true because we did, in fact, parse a number. | ||
Comment on lines
+317
to
320
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is "just in case". I can't think of any actual scenario, other octal or hex literal (handled above), where |
||
} | ||
*output = value; | ||
input_->Next(); | ||
return true; | ||
} else if (LookingAt("inf")) { | ||
|
@@ -1551,18 +1567,20 @@ bool Parser::ParseOption(Message* options, | |
is_negative | ||
? static_cast<uint64_t>(std::numeric_limits<int64_t>::max()) + 1 | ||
: std::numeric_limits<uint64_t>::max(); | ||
DO(ConsumeInteger64(max_value, &value, "Expected integer.")); | ||
if (is_negative) { | ||
value_location.AddPath( | ||
UninterpretedOption::kNegativeIntValueFieldNumber); | ||
uninterpreted_option->set_negative_int_value( | ||
static_cast<int64_t>(0 - value)); | ||
} else { | ||
value_location.AddPath( | ||
UninterpretedOption::kPositiveIntValueFieldNumber); | ||
uninterpreted_option->set_positive_int_value(value); | ||
if (TryConsumeInteger64(max_value, &value)) { | ||
if (is_negative) { | ||
value_location.AddPath( | ||
UninterpretedOption::kNegativeIntValueFieldNumber); | ||
uninterpreted_option->set_negative_int_value( | ||
static_cast<int64_t>(0 - value)); | ||
} else { | ||
value_location.AddPath( | ||
UninterpretedOption::kPositiveIntValueFieldNumber); | ||
uninterpreted_option->set_positive_int_value(value); | ||
} | ||
break; | ||
} | ||
break; | ||
// value too large for an integer; fall through below to treat as floating point | ||
} | ||
|
||
case io::Tokenizer::TYPE_FLOAT: { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1002,9 +1002,19 @@ bool Tokenizer::ParseInteger(const std::string& text, uint64_t max_value, | |
} | ||
|
||
double Tokenizer::ParseFloat(const std::string& text) { | ||
double result; | ||
jhump marked this conversation as resolved.
Show resolved
Hide resolved
|
||
GOOGLE_LOG_IF(DFATAL, | ||
!TryParseFloat(text, &result)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hate side effected in LOG statements, do you mind switching this to the more straight forward:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
<< " Tokenizer::ParseFloat() passed text that could not have been" | ||
" tokenized as a float: " | ||
<< absl::CEscape(text); | ||
return result; | ||
} | ||
|
||
bool Tokenizer::TryParseFloat(const std::string& text, double* result) { | ||
const char* start = text.c_str(); | ||
char* end; | ||
double result = NoLocaleStrtod(start, &end); | ||
*result = NoLocaleStrtod(start, &end); | ||
|
||
// "1e" is not a valid float, but if the tokenizer reads it, it will | ||
// report an error but still return it as a valid token. We need to | ||
|
@@ -1020,12 +1030,7 @@ double Tokenizer::ParseFloat(const std::string& text) { | |
++end; | ||
} | ||
|
||
GOOGLE_LOG_IF(DFATAL, | ||
static_cast<size_t>(end - start) != text.size() || *start == '-') | ||
<< " Tokenizer::ParseFloat() passed text that could not have been" | ||
" tokenized as a float: " | ||
<< absl::CEscape(text); | ||
return result; | ||
return static_cast<size_t>(end - start) == text.size() && *start != '-'; | ||
} | ||
|
||
// Helper to append a Unicode code point to a string as UTF8, without bringing | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put this here, instead of just relying on the branch below, because I think an octal literal would likely be successfully parsed by
ParseFloat
, but incorrectly interpreted as decimal.