Bug 143121 - Respect cache-control directives in request
Summary: Respect cache-control directives in request
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 143159
  Show dependency treegraph
 
Reported: 2015-03-26 16:43 PDT by Antti Koivisto
Modified: 2015-03-27 15:39 PDT (History)
4 users (show)

See Also:


Attachments
patch (31.24 KB, patch)
2015-03-26 16:51 PDT, Antti Koivisto
cdumez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2015-03-26 16:43:36 PDT
We should respect Cache-control: no-cache, no-store, max-age=0 in the request too.

https://tools.ietf.org/html/rfc7234#section-5.2.1
Comment 1 Antti Koivisto 2015-03-26 16:47:35 PDT
rdar://problem/19714040
Comment 2 Antti Koivisto 2015-03-26 16:51:39 PDT
Created attachment 249543 [details]
patch
Comment 3 WebKit Commit Bot 2015-03-26 16:54:07 PDT
Attachment 249543 [details] did not pass style-queue:


ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:188:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Chris Dumez 2015-03-26 17:09:52 PDT
Comment on attachment 249543 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249543&action=review

r=me

> Source/WebCore/loader/cache/CacheValidation.cpp:155
> +    // See RFC 2616, Section 2.2

We may want to point to the new RFC instead of the deprecated one since we're moving this code :)

> Source/WebCore/loader/cache/CacheValidation.cpp:182
> +static bool isControlCharacter(UChar c)

We could probably mark this one as inline.

> Source/WebCore/loader/cache/CacheValidation.cpp:256
> +            // RFC2616 14.9.1: A no-cache directive with a value is only meaningful for proxy caches.

Ditto about outdated RFC.

> Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:186
> +        auto requestCacheControl = WebCore::parseCacheControlDirectives(request.httpHeaderFields());

I feel that this variable should have 'request' somewhere in the name to avoid confusion with the response ones.

> Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:285
> +    auto cacheControlDirectives = WebCore::parseCacheControlDirectives(originalRequest.httpHeaderFields());

I feel that this variable should have 'request' somewhere in the name to avoid confusion with the response ones.
Comment 5 Chris Dumez 2015-03-26 17:10:30 PDT
But please fix build for other ports :)
Comment 6 Antti Koivisto 2015-03-27 08:14:18 PDT
https://trac.webkit.org/r182059
Comment 7 Csaba Osztrogonác 2015-03-27 09:44:19 PDT
(In reply to comment #5)
> But please fix build for other ports :)

Done in https://trac.webkit.org/changeset/182063
Comment 8 Chris Dumez 2015-03-27 09:45:33 PDT
(In reply to comment #7)
> (In reply to comment #5)
> > But please fix build for other ports :)
> 
> Done in https://trac.webkit.org/changeset/182063

I meant Antti but thanks Ossy for taking care of it.
Comment 9 Antti Koivisto 2015-03-27 12:36:08 PDT
> I meant Antti but thanks Ossy for taking care of it.

I believe I fixed it in https://trac.webkit.org/r182064 .