Closed
Bug 1408477
Opened 7 years ago
Closed 7 years ago
Add Cache-Control, Pragma headers in tc-lib-app or maybe tc-lib-api
Categories
(Taskcluster :: Services, enhancement)
Taskcluster
Services
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: dustin, Assigned: dustin)
References
Details
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•7 years ago
|
||
It seems weird that for idempotent requests you would want to disable caching.
Assignee | ||
Comment 2•7 years ago
|
||
Yeah, I need to look at what we're doing now and what we want to do.
Assignee | ||
Comment 3•7 years 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)
Comment 4•7 years ago
|
||
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•7 years ago
|
||
#2 it is!
Assignee | ||
Comment 6•7 years ago
|
||
Assignee | ||
Comment 7•7 years ago
|
||
The `Pragma` header is only checked for no-cache if it exists, so no need to do anything there.
Comment 8•7 years 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•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Component: Platform Libraries → Services
You need to log in
before you can comment on or make changes to this bug.
Description
•