File server missing Vary on 206, 304

[dkarlovi]

I'm checking my caching headers for assets served by file server. It all works as expected, but RedBot claims it should be adding Vary when serving 206, 304.

I've checked Caddy site itself that it's not some misconfiguration on my end, but it doesn't seem to be, the case there seems even worse:
https://redbot.org/?uri=https://caddyserver.com/resources/css/docs.css?v=dfd5278

It seems the Vary needs to be sent in both cases.

[mholt]

Hm, interesting. I didn't know that (guess I didn't read the spec.)

I'm currently attending to a hospital stay and will be delayed with my work a bit. Is there anyone who would like to pick this up?

[dkarlovi]

@mholt I can try, but I'm a Golang newbie and will probably need a bit of guidance, it that's not too annoying?

[mholt]

@dkarlovi No that should be fine! I am a bit swamped lately (as you can see I'm behind on issues and PRs) -- with a hospital stay I'm attending to, but I will do my best to offer feedback.

If it ends up being too much, it's OK to let someone else take it on. But you're certainly welcome!

[dkarlovi]

@mholt thanks, I'll try to take a stab at it, it's no rush since nobody complained until now. 😆

Hope the hospital sees the back of you ASAP, all the best with that, take care.

[francislavoie]

I think this might be pretty tricky to solve, because Caddy's file_server and encode are quire detached from eachother. Plus, some of the headers are written by the Go stdlib itself, and aren't controlled by Caddy directly. I don't think it'll be impossible though, just quite tricky to get right.

[fortum-vaanavil]

I could also take a stab at this since I've lately been digging into file_server quite a bit. @dkarlovi , what's your situation here?

[dkarlovi]

@fortum-vaanavil TBH since I've read @francislavoie comment about it being non-trivial, I got discouraged about being able to do it so please feel free to take the lead here, thank you!

[francislavoie]

Sorry 😓 didn't mean to be discouraging, just wanted to manage expectations.

PRs definitely welcome. But make sure to take care to not break abstractions (encode and file_server must not depend on eachother).

[dkarlovi]

No problems, I didn't interpret it as hostile, on the contrary: you've warned this might not be the best place for me to try getting into Go (and Caddy's codebase) so I'll find some other place to contribute, don't worry. 👍 Thanks!

[mholt]

@dkarlovi Can you provide a little more information about the kinds of requests and responses that have these validation errors?

Preferably a minimal config and some curl -v commands that can reproduce the scenarios tested by your validation tool.

That said, I believe I have a commit that should fix your initial report if I guessed right. I did my best to reproduce a likely scenario locally and verified that my browser was still able to use its cached values with the patched logic. Could you please try the latest on master to confirm?

Thanks!