Bug 143121

Summary: Respect cache-control directives in request
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, japhet, ossy
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 143159    
Attachments:
Description Flags
patch cdumez: review+

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 .