Comments (7)
Actually, the question is if iodine should add the Last-Modified header to a response it knows nothing about at all, giving the client the option to send the conditional request when the application didn't intend this. But in the ETag case it is definitely wrong.
from iodine.
Hi @raxoft ,
Thank you for opening this issue.
A quick question about a workaround: will setting the Last-Modified
header manually to a correct value not solve the issue? (I will test this later, but Iodine should only add the Last-Modified
header if it is missing)...
I will read the RFC again to understand the issue better, as I might not understand the situation as well I would have hoped to (it's been a while since I reviewed the specifications).
You are right to ask if it is helpful for iodine to supply default header values when developers don't supply them. However, changing this will be a breaking change for those who currently rely on the default behavior (unless the default behavior is a bug, in which case change is good)...
...currently the default behavior assumes that dynamic requests shouldn't be cached.
To re-asses the default behavior, may I ask a few questions about your use case?
-
Why do you use the
Rack::ETag
andRack::ConditionalGet
middleware? Are you serving static data using Rack (rather than a proxy / using iodine's static server)? -
Is a
Last-Modified
header data unavailable to you (i.e., is theetag
header a hash of the resulting dynamic data)?
Thanks again.
Bo.
from iodine.
I've been reading more and I think you are right about the Last-Modified
header... it probably should not be set by default.
I was previously misreading the RFC where it stated that:
HTTP/1.1 origin servers: ... SHOULD send a Last-Modified value if it is feasible to send one...
I was ignoring the rest of the sentence where they write:
unless the risk of a breakdown in semantic transparency that could result from using this date in an If-Modified-Since header would lead to serious problems.
I'd still love to read more about your use-case to better understand how I can improve iodine... but it's probably better to remove the default Last-Modified
header anyway.
Thanks again!
from iodine.
Hi, thanks for looking at this so quickly.
First to your original question - adding Last-Modified to the response is not practical, because if I would do that, it would disable Rack::ETag from doing anything, since it checks both Last-Modified and ETag headers and doesn't add ETag if one is present. That would essentially mean I would have to implement ETag myself, which kind of defeats the purpose of the middleware.
As for the use case itself - we use the ETag for dynamic responses from our API where the content may remain unchanged, but we can't rely on timestamps which have just one second precision. The data we transfer are large enough that it pays off to provide the ETag to avoid retransmitting the data the client already has in its cache.
Overall, I agree with the conlusion you came to - I think that adding the Last-Modified header to dynamic responses in general can cause troubles. In my case it prevents caching, but without ETag, it could actually cause false positive hits in hierarchy of caches. Let's the application provide this header if it wants (for example, sinatra has nice support for both Last-Modified and ETag headers which exit early IIRC in case of a match) and not mess with it at all at iodine's level.
Thanks.
from iodine.
Hi @raxoft ,
I pushed a fix and was wondering if you wanted to test it or request any more changes before I publish a release and close the issue...?
Kindly,
Bo.
P.S.
As for the use case itself - we use the ETag for dynamic responses from our API where the content may remain unchanged...
Would it make sense for you if Iodine itself tested the response's etag
against the request's if-none-match
or implemented some sort of callback to that effect (in effect, pulling the etag
middleware logic into the server / C layer)?
I am starting to think that for version 0.8.x automatic recognition of cached responses could improve server performance by minimizing memory usage and avoiding system calls (write
) for responses that were cached by the client.
Adding a callback could drop requests before the response is generated, freeing up more user resources.
I didn't have this use case for dynamic responses, but it makes sense to me that others might.
from iodine.
Thanks for quick fix. Tested it right now, works great as expected.
BTW, seeing the diff, in the hex_val
macro, you can do the (c) | 32
trick already in the condition, saving you two tests and couple jumps.
Regarding moving the conditional get functionality into iodine itself... Hmm, it's kind of a tricky question. If you moved the functionality of both middlewares to iodine, it might help speeding up the etag generation. OTOH, it might complicate things so much that it will just cause extra overhead for those who don't need it.
Other than that, I don't think it's that useful (that is, if you were to move just the conditional get part alone). The biggest savings when using these headers come from not having to generate the response in the first place when possible, and that's already possible to do in the application. For example, sinatra has the last_modified
and etag
helpers which not only set the response headers, but halt with 304 immediately in case of a match, allowing you to skip bulk of the response generation. Of course, it assumes that you have to have some reliable value available which you can use for these validators. For example, for generated but static pages, you can use the app start time with the last_modified
helper. But for truly dynamic responses it is often too complicated to generate something like that ahead of the actual content generation, thus just generating the content and computing the etag from that is usually the way to go.
Also, if you were to move the etag generation to iodine, the configurability of the whole stack would become tricky. I mean, with middleware you can easily add etag generation just for some routes in the stack, but now it would either be all or nothing, or perhaps require some special headers meant only for iodine to control that (on top of the Cache-Control headers), which sounds too complicated.
In either case, in my opinion it would be a completely new feature, so I think you can close this issue for now and add this feature to your 'think-about-it' list instead.
Thanks.
from iodine.
Hi @raxoft ,
Thanks for the thought out response and the tips.
I just released the update.
As for the new feature, I will think about it. Yes, it's much better if the app performs its own tests in advance, but I think the server could save some bandwidth and memory by testing if response.headers["etag"] == request.headers["etag"]
(even if the logic is more than that simple conditional)... so far I was simply saving cached data to disk and allowing the static server to deal with the caching validation (this had the additional benefits related to how much of the data was live in memory).
Anyway, thanks again!
from iodine.
Related Issues (20)
- Iodine failing to compile with Ruby 3.2.0preview_2 HOT 5
- The future of iodine (vs. plezi)...? HOT 9
- [Rack 3] Response headers with Array value should produce multiple header fields HOT 3
- CPU usage when idle HOT 3
- Iodine.unsubscribe crashes when called with symbol. HOT 4
- Subscribing from the enter_master block subscribes even the children. HOT 3
- unix socket not writeable due umask HOT 8
- Writes to STDOUT even when Iodine.verbosity = 0 HOT 1
- Does not honor HUP signal. HOT 1
- Iodine stops on TERM signal with error when it run with -pid key and more than one worker HOT 1
- The Fixnum is gone as of Ruby-2.4 HOT 3
- response code 418 results in 500 Internal Server Error HOT 1
- [FEAT] return reason for closed connection/client HOT 7
- Static file serving: allow mounting of multiple local dirs HOT 14
- binary C extension not built? HOT 26
- How to get client instance id HOT 3
- ondisconnect example HOT 1
- rage-rb HOT 1
- undefining the allocator of T_DATA class IodineObjectStorage at startup HOT 2
- Iodine failing to build in fedora 40 HOT 5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from iodine.