Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cache headers to POSIX version too #416

Merged
merged 2 commits into from
Dec 12, 2024

Conversation

lapo-luchini
Copy link
Contributor

Add cache headers to POSIX version too.

Copy link
Collaborator

@AlCutter AlCutter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Just a small nit/question inline.

cmd/conformance/posix/main.go Outdated Show resolved Hide resolved
@lapo-luchini
Copy link
Contributor Author

This has the problem of marking immutable also the /tile/ sub-folders (which contain a list of files which is mutable).
I'm not sure this is a real problem, but might need some more complex path matching. (note that some of the headers wre added by my reverse proxy)

% xh localhost:2025/tile/0/
HTTP/1.1 200 OK
Cache-Control: public, max-age=31536000, immutable
Content-Type: text/html; charset=utf-8
Date: Thu, 12 Dec 2024 15:21:54 GMT
Last-Modified: Mon, 09 Dec 2024 22:47:14 GMT
Transfer-Encoding: chunked

<pre>
<a href="000">000</a>
<a href="000.p/">000.p/</a>
<a href="001">001</a>
<a href="001.p/">001.p/</a>
<a href="002">002</a>
<a href="002.p/">002.p/</a>
[omissis]
</pre>

@AlCutter
Copy link
Collaborator

This has the problem of marking immutable also the /tile/ sub-folders (which contain a list of files which is mutable). I'm not sure this is a real problem, but might need some more complex path matching. (note that some of the headers wre added by my reverse proxy)

% xh localhost:2025/tile/0/
HTTP/1.1 200 OK
Cache-Control: public, max-age=31536000, immutable
Content-Type: text/html; charset=utf-8
Date: Thu, 12 Dec 2024 15:21:54 GMT
Last-Modified: Mon, 09 Dec 2024 22:47:14 GMT
Transfer-Encoding: chunked

<pre>
<a href="000">000</a>
<a href="000.p/">000.p/</a>
<a href="001">001</a>
<a href="001.p/">001.p/</a>
<a href="002">002</a>
<a href="002.p/">002.p/</a>
[omissis]
</pre>

Those partial tiles (.p/...) are mutable only in the sense that the tlog-tiles spec permits the log implementation to later remove them once a full tile at the same coordinates is present, so I think the only issue this causes is that a cache may hang on to partial tile resources longer than it strictly needs to.

@lapo-luchini
Copy link
Contributor Author

I think the only issue this causes is that a cache may hang on to partial tile resources longer than it strictly needs to.

On a fresh install I onlt see these elements, tough, but soon some 006 and 007.p will show up. (but hidden until the reverse proxy cache expires)

<pre>
<a href="000">000</a>
<a href="000.p/">000.p/</a>
<a href="001">001</a>
<a href="001.p/">001.p/</a>
<a href="002">002</a>
<a href="002.p/">002.p/</a>
<a href="003">003</a>
<a href="003.p/">003.p/</a>
<a href="004">004</a>
<a href="004.p/">004.p/</a>
<a href="005">005</a>
<a href="005.p/">005.p/</a>
<a href="006.p/">006.p/</a>
</pre>

@AlCutter
Copy link
Collaborator

I think the only issue this causes is that a cache may hang on to partial tile resources longer than it strictly needs to.

On a fresh install I onlt see these elements, tough, but soon some 006 and 007.p will show up. (but hidden until the reverse proxy cache expires)

<pre>
<a href="000">000</a>
<a href="000.p/">000.p/</a>
<a href="001">001</a>
<a href="001.p/">001.p/</a>
<a href="002">002</a>
<a href="002.p/">002.p/</a>
<a href="003">003</a>
<a href="003.p/">003.p/</a>
<a href="004">004</a>
<a href="004.p/">004.p/</a>
<a href="005">005</a>
<a href="005.p/">005.p/</a>
<a href="006.p/">006.p/</a>
</pre>

Right - if you delete the log and later create a brand new log served at the same URL via a cache it's going to be not good on various fronts :)

This shouldn't ever happen with a production log (one URL <-> one log), but I'm wondering if we should lower those max-age values in these conformance binaries to limit damage if someone does blindly copy & paste.

I think we can merge this now, at least it brings all the conformance FEs into line, and then figure out whether to revisit the age.

@AlCutter AlCutter merged commit e7adf39 into transparency-dev:main Dec 12, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants