Bug 1940748

Summary:	Status Badges allow too permissive caching
Product:	[Community] Copr	Reporter:	Chris Bouchard <chris>
Component:	backend	Assignee:	Silvie Chlupova <schlupov>
Status:	CLOSED NEXTRELEASE	QA Contact:
Severity:	low	Docs Contact:
Priority:	unspecified
Version:	unspecified	CC:	dbranchini, emanuele, praiskup, schlupov
Target Milestone:	---	Keywords:	Reopened
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2021-07-15 09:55:00 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Chris Bouchard 2021-03-19 04:32:07 UTC

Description of problem:

The headers for status badge images allow the image to be cached for too long (currently up to 12 hours), which leads sites like GitHub (which cache all images from rendered markdown documents) to display out-of-date badges. The headers for status badges should either prevent caching all together, or at least specify a shorter window (on the order of minutes).


Version-Release number of selected component (if applicable): n/a


How reproducible: Consistently


Steps to Reproduce:
1. Obtain the URL for the status badge of a package, e.g., https://copr.fedorainfracloud.org/coprs/chrisbouchard/upliftinglemma/package/namespaced-wireguard-vpn/
2. Request the URL.
3. Inspect the Cache-Control header.

Actual results:

"Cache-Control: public, max-age=43200", where 43200 seconds = 12 hours.

Expected results:

Either "Cache-Control: no-cache" or, e.g., "Cache-Control: public, max-age=300", where 300 seconds = 5 minutes.


Additional info:

Setting "Cache-Control" also assumes the ETag header correctly represents the image contents (i.e., that it changes when the image changes).

Comment 1 Chris Bouchard 2021-03-19 04:35:22 UTC

GitHub's aggressive caching has been documented in an issue: https://github.com/github/markup/issues/224

According to user bkeepers in that issue discussion, they think their caching implementation is correct and don't plan to change it.

Comment 2 Pavel Raiskup 2021-03-19 06:50:46 UTC

We already have the no-cache header configured:
https://pagure.io/copr/copr/blob/a13faa9be29005f68581ba6dc9a6c8e9427aa206/f/frontend/coprs_frontend/coprs/views/misc.py#_380

But it apparently stopped working:
$ curl -v https://copr.fedorainfracloud.org/coprs/g/copr/copr-dev/package/copr-frontend/status_image/last_build.png 2>&1 >/dev/null | grep -i cache
< Cache-Control: public, max-age=43200

Thank you for the report!

Comment 3 Pavel Raiskup 2021-03-19 06:57:43 UTC

No, the no-cache header is only added when it makes any sense - ie when the
build state is going to be changed...  OTOH if the build already failed or
succeeded, nothing can actually change the state anymore and thus neither
the icon -- and it is valid to have it cached.

From one of the currently running builds:
$ curl -v https://copr.fedorainfracloud.org/coprs/build/2081848/status_image.png 2>&1 >/dev/null | grep -i cache
< Cache-Control: no-cache

This seems to be OK to me, but feel free to reopen and add more info.

Comment 4 Chris Bouchard 2021-03-19 12:33:21 UTC

Comment 5 Chris Bouchard 2021-03-19 12:38:01 UTC

Pavel, that makes sense for any individual build's status. The problem is when it's accessed via the package's `last_build.png` image (alias?), which IMO should *never* be cached because there could always be another build.

PS. You mentioned to "feel free to reopen and add more info", but I'm new to Bugzilla. Should I change the status back to NEW?

Comment 6 Pavel Raiskup 2021-03-19 12:45:15 UTC

Ah, indeed!  Thank you for the additional info.  We need to better take care of the latest build batch.

Comment 7 Silvie Chlupova 2021-07-15 09:55:00 UTC

Fixed by commit https://pagure.io/fork/schlupov/copr/copr/c/1dfabf95df0f23d6f4aa5dc57aeb965051a01e62.