kilork / actix-web-static-files

actix-web static files as resources support
The Unlicense
75 stars 18 forks source link

Expose API to set cache-control and other headers #58

Open steadygaze opened 4 months ago

steadygaze commented 4 months ago

Hi, I found this package useful; thanks for maintaining it!

I was looking at the code in this crate, and I noticed that it should be setting ETags, but they aren't showing up at all when I look at the headers in the browser devtools or curl. This is despite the content-type being detected and set correctly. Example:

HTTP/1.1 200 OK
content-length: 14995
content-type: text/css
date: Sun, 02 Jun 2024 04:48:54 GMT

This seems like a bug to me. Looking at the code in respond_to, I don't see how it could both set the content-type but not the etag, so I'm not sure how to fix it. EDIT: I think it's related to my project setup somehow; I'll keep debugging it.

Additionally, I was looking through the API docs and I realized there is no way to set the headers that are sent back, specifically the cache-control headers, which are complimentary to etags.

I was looking at the code and it seemed like somewhere in respond_to would be the right place to set custom headers. Probably the simplest way to implement this is to take a static list of headers to inject, but it would probably be more powerful to add an API (to ResourceFiles) that takes a lambda that is given the file and the in-progress HttpResponseBuilder and can do whatever it wants.

steadygaze commented 4 months ago

I did some printf debugging and found that it is apparently exercising the code to set the ETag header properly, so I'm not sure what's happening there. I tried without any middlewares and without systemfd as well.

I'll try a minimal repro tomorrow.

steadygaze commented 4 months ago

I noticed the example works just fine, so it must be something in my project's setup. I'll keep debugging it.

kilork commented 4 months ago

Hey @steadygaze Check your Browser settings, I often activate "No cache" option in Firefox during dev. In curl you need -v option.

Or maybe it is really something very specific for your configuration. Anyway, thank you for reporting!

steadygaze commented 4 months ago

Yeah, thanks for the suggestions; I discovered a a day or two later that it was actually just one of my custom routes. I was under the mistaken impression that one of my routes was managed by this crate. (So, it was my own stupidity/forgetfulness 🤦😆.)

What I was trying to do is to run tailwindcss to generate a CSS file for inclusion, while simultaneously I wanted to include some static files. I didn't really understand how to do two things simultaneously, so I was outputting to the OUT_DIR and manually creating a route just for that. I changed it to dump to the same static directory instead, so it can be managed by this crate with the rest of the static files. I wasn't sure if using generated files outside of OUT_DIR would cause problems, but I realized when looking at the output generated by static files that that's what it's doing already anyway. I have noticed sometimes that my project fails to build after renaming/moving a file, and the only workaround is to run cargo clean and rebuild the entire thing, which is possibly related.

If it's not considered too trivial, I can probably contribute what I did to the examples repo when I get the chance.

Related to being able to set cache-control headers, I don't really need the feature that badly right now to be honest, but when I get around to it, I'd happily contribute it.