New issue
Advanced search Search tips

Issue 596551 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

HttpResponseHeaders::GetCacheControlDirective() doesn't check the return value from StrintToInt64

Project Member Reported by eroman@chromium.org, Mar 21 2016

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 }
 

Comment 1 by eroman@chromium.org, Mar 21 2016

Components: -Internals>Network Internals>Network>HTTP
It may be that StringToInty64() sets a consistent value on failure, but I don't see that called out in its documentation (just says it will write to output, not what that value is).

Comment 2 by mmenke@chromium.org, May 30 2018

Components: -Internals>Network>HTTP Internals>Network>Cache
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.
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