HttpResponseHeaders::GetCacheControlDirective() doesn't check the return value from StrintToInt64 |
||
Issue description
It looks like the code below does not check whether StringToInt() was successful, and may assign an uninitialized value to *result....
753 bool HttpResponseHeaders::GetCacheControlDirective(const StringPiece& directive,
754 TimeDelta* result) const {
755 StringPiece name("cache-control");
756 std::string value;
757
758 size_t directive_size = directive.size();
759
760 size_t iter = 0;
761 while (EnumerateHeader(&iter, name, &value)) {
762 if (value.size() > directive_size + 1 &&
763 base::StartsWith(value, directive,
764 base::CompareCase::INSENSITIVE_ASCII) &&
765 value[directive_size] == '=') {
766 int64_t seconds;
767 base::StringToInt64(
768 StringPiece(value.begin() + directive_size + 1, value.end()),
769 &seconds);
770 *result = TimeDelta::FromSeconds(seconds);
771 return true;
772 }
773 }
774
775 return false;
776 }
,
May 30 2018
StringToInt64 does have well-defined behavior on errors. No idea if they're the ones we want, though. This seems more a cache issue than an HTTP issue. I'll defer to Maks on whether this is worth looking into.
,
May 30 2018
https://cs.chromium.org/chromium/src/base/strings/string_number_conversions.h?rcl=115ba21da04c63f6c5eb47f2f333f401737c1c68&l=84 lists out what it does in various failure conditions. Ther overflow one does actually match https://tools.ietf.org/html/rfc7234#section-1.2.1 but I am not really sure if anything specifies proper behavior in all other cases; seems like not even the new RFCs are quite that precise. |
||
►
Sign in to add a comment |
||
Comment 1 by eroman@chromium.org
, Mar 21 2016