Skip to content

Commit

Permalink
css2: Fix assertion failure on very large numbers
Browse files Browse the repository at this point in the history
The spec doesn't mention any kind of precision, so let's just clamp
things to the int32_t range for now.
  • Loading branch information
robinlinden committed Oct 8, 2024
1 parent cd937f1 commit 9f9abab
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 7 deletions.
2 changes: 1 addition & 1 deletion css2/token.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ struct DelimToken {
};

struct NumberToken {
std::variant<int, double> data;
std::variant<std::int32_t, double> data;
[[nodiscard]] bool operator==(NumberToken const &) const = default;

[[nodiscard]] constexpr bool is_integer() const { return std::holds_alternative<int>(data); }
Expand Down
26 changes: 21 additions & 5 deletions css2/tokenizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
#include "util/from_chars.h"
#include "util/string.h"

#include <algorithm>
#include <cassert>
#include <charconv>
#include <cstdint>
#include <limits>
#include <optional>
#include <string>
#include <system_error>
Expand Down Expand Up @@ -392,8 +394,8 @@ void Tokenizer::reconsume_in(State state) {
}

// https://www.w3.org/TR/css-syntax-3/#consume-a-number
std::variant<int, double> Tokenizer::consume_number(char first_byte) {
std::variant<int, double> result{};
std::variant<std::int32_t, double> Tokenizer::consume_number(char first_byte) {
std::variant<std::int32_t, double> result{};
std::string repr{};

assert(util::is_digit(first_byte) || first_byte == '-' || first_byte == '+' || first_byte == '.');
Expand Down Expand Up @@ -428,14 +430,28 @@ std::variant<int, double> Tokenizer::consume_number(char first_byte) {
// TODO(robinlinden): Step 5

// The tokenizer will verify that this is a number before calling consume_number.
//
// The spec doesn't mention precision of this, so let's clamp it to the
// int32_t range for now.
[[maybe_unused]] util::from_chars_result fc_res{};
if (auto *int_res = std::get_if<int>(&result); int_res != nullptr) {
if (auto *int_res = std::get_if<std::int32_t>(&result); int_res != nullptr) {
fc_res = util::from_chars(repr.data(), repr.data() + repr.size(), *int_res);
*int_res = std::clamp(
*int_res, std::numeric_limits<std::int32_t>::min(), std::numeric_limits<std::int32_t>::max());
} else {
fc_res = util::from_chars(repr.data(), repr.data() + repr.size(), std::get<double>(result));
auto &dbl_res = std::get<double>(result);
fc_res = util::from_chars(repr.data(), repr.data() + repr.size(), dbl_res);
dbl_res = std::clamp(dbl_res,
static_cast<double>(std::numeric_limits<std::int32_t>::min()),
static_cast<double>(std::numeric_limits<std::int32_t>::max()));
}

if (fc_res.ec == std::errc::result_out_of_range) {
result = repr[0] == '-' ? std::numeric_limits<std::int32_t>::min() : std::numeric_limits<std::int32_t>::max();
} else {
assert(fc_res.ec == std::errc{} && fc_res.ptr == repr.data() + repr.size());
}

assert(fc_res.ec == std::errc{} && fc_res.ptr == repr.data() + repr.size());
return result;
}

Expand Down
2 changes: 1 addition & 1 deletion css2/tokenizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class Tokenizer {
bool is_eof() const;
void reconsume_in(State);

std::variant<int, double> consume_number(char first_byte);
std::variant<std::int32_t, double> consume_number(char first_byte);
std::string consume_an_escaped_code_point();
};

Expand Down
21 changes: 21 additions & 0 deletions css2/tokenizer_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "etest/etest2.h"

#include <limits>
#include <source_location>
#include <string>
#include <string_view>
Expand Down Expand Up @@ -377,6 +378,16 @@ int main() {
expect_token(output, CloseParenToken{});
});

s.add_test("integer: large", [](etest::IActions &a) {
auto output = run_tokenizer(a, "12147483647");
expect_token(output, NumberToken{std::numeric_limits<std::int32_t>::max()});
});

s.add_test("integer: large negative", [](etest::IActions &a) {
auto output = run_tokenizer(a, "-12147483648");
expect_token(output, NumberToken{std::numeric_limits<std::int32_t>::min()});
});

s.add_test("integer: leading 0", [](etest::IActions &a) {
auto output = run_tokenizer(a, "00000001");
expect_token(output, NumberToken{.data = 1});
Expand Down Expand Up @@ -452,6 +463,16 @@ int main() {
expect_token(output, DelimToken{'.'});
});

s.add_test("number: large", [](etest::IActions &a) {
auto output = run_tokenizer(a, "12147483647.0");
expect_token(output, NumberToken{static_cast<double>(std::numeric_limits<std::int32_t>::max())});
});

s.add_test("number: large negative", [](etest::IActions &a) {
auto output = run_tokenizer(a, "-12147483648.0");
expect_token(output, NumberToken{static_cast<double>(std::numeric_limits<std::int32_t>::min())});
});

s.add_test("number: no digits before decimal point", [](etest::IActions &a) {
auto output = run_tokenizer(a, ".25");
expect_token(output, NumberToken{.25});
Expand Down

0 comments on commit 9f9abab

Please sign in to comment.