[Webkit-unassigned] [Bug 24656] cacheControlContainsNoCache() in ResourceResponseBase.h is wrong

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=24656





------- Comment #8 from ddkilzer at webkit.org  2009-03-29 10:05 PDT -------
(In reply to comment #7)
> (In reply to comment #6)
> 
> > >          if ((equalIgnoringCase(directives[i].first, "private") || equalIgnoringCase(directives[i].first, "no-cache")) && !directives[i].second.isEmpty())
> > >              parseCacheControlDirectiveValues(directives[i].second, directiveValues);
> > >          else
> > > -            directiveValues.append(directives[i].second);
> > > +            directiveValues.append(directives[i].first);
> > 
> > This code looks busted in many more ways, and I don't think that it's very
> > desirable to only fix one minor case. All this check does is stuff
> > Cache-Control private and no-cache directive values into one bucket with other
> > directives, which looks bogus. E.g., one possible use of this syntax is
> > 'Cache-Control: no-cache="X-Foobar"', which means that 'X-Foobar' header must
> > not be sent in a proxy response without revalidation.
> 
> The "private" and "no-cache" directive values are NOT put into a single bucket.
>  You must read the second if/else statement below it to see how the
> directiveValues data is used.  The check for "private" or "no-cache" in the
> code above is because the spec says that they are the only directives that may
> have one or more sub-values, so they are parsed differently than other
> directives.  (But the code inside the outer for loop handles directives with
> and without sub-values, which may be why it's confusing.)

I remember now, the parsing code was changed in r39383 because fully parsing
the cache-control headers was a memory regression:

<http://trac.webkit.org/changeset/39383>

The original code I committed in r38145 did do correct parsing of those
headers.

-            directiveValues.append(directives[i].second);
+            directiveValues.append(directives[i].first);

This change is incorrect.

-        return m_cacheControlContainsMustRevalidate;
+        return m_cacheControlContainsNoCache;

This change is correct and fixes a copy-paste error in
cacheControlContainsNoCache().

If more detail is needed for Cache-Control headers (after r39383), you'll have
to modify the parsing routines to check for what you're looking for and stuff
it into a 1-bit bool field or return the data on an as-needed basis.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.