Add Cache-Control, Pragma headers in tc-lib-app or maybe tc-lib-api

RESOLVED FIXED

Status

Taskcluster
Platform Libraries
RESOLVED FIXED
a month ago
23 days ago

People

(Reporter: dustin, Assigned: dustin)

Tracking

(Blocks: 2 bugs)

Details

(Assignee)

Description

a month ago
https://github.com/zaproxy/zap-extensions/blob/ba87c70bfa83b24d01999d8e5b6eb98dcab19441/src/org/zaproxy/zap/extension/pscanrules/CacheControlScanner.java

This basically requires no caching.  Are we OK with that for all of our API methods?

Comment 1

a month ago
It seems weird that for idempotent requests you would want to disable caching.
(Assignee)

Comment 2

a month ago
Yeah, I need to look at what we're doing now and what we want to do.
(Assignee)

Comment 3

29 days ago
We don't currently set any cache control headers in tc-lib-app or tc-lib-api.

This ZAP check would require that it be set to 'no-store no-cache must-revalidate'.  The default when the cache-control header is omitted is "private", which is not the same as "no-cache":

> Indicates that the response is intended for a single user and must not be stored by a shared cache.
> A private cache may store the response.

So the options are:

 1) Change nothing in services, ask that ZAP stop making this particular check for our APIs
 2) Set no-store no-cache must-revalidate unconditionally for all API methods
 3) Set more detailed cache headers for some GET requests (but not all..)

Jonas, thoughts?  I don't want to make this a project, so IMHO the choice is between 1 and 2, in other words between "private" and "no-store no-cache must-revalidate".
Flags: needinfo?(jopsen)
I agree with Eli that it's weird to forbid caching.
We could plausibly infer "public" from whether scopes is required.

But since payloads are small and cost of checking if the data have changed is equal to fetching, I suggest we just do:
  'no-store no-cache must-revalidate'
That's is definitely safe, setting a better cache header is an optimization we can do that some other day.
Flags: needinfo?(jopsen)
(Assignee)

Comment 5

25 days ago
#2 it is!
(Assignee)

Comment 6

23 days ago
https://github.com/taskcluster/taskcluster-lib-api/pull/64
(Assignee)

Comment 7

23 days ago
The `Pragma` header is only checked for no-cache if it exists, so no need to do anything there.

Comment 8

23 days ago
Commits pushed to master at https://github.com/taskcluster/taskcluster-lib-api

https://github.com/taskcluster/taskcluster-lib-api/commit/2afd4a3fc7fb7fbf1bbf5d8f4d09a4d9e2aece42
Bug 1408477 - always set cache-control

https://github.com/taskcluster/taskcluster-lib-api/commit/f6dc47207024cacc1d7ab248f1e7494b66359082
Merge pull request #65 from taskcluster/bug1408477

Bug 1408477 - always set cache-control
(Assignee)

Updated

23 days ago
Status: NEW → RESOLVED
Last Resolved: 23 days ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.