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

Last-Modifed and If-Modified-Since handling with responseFile #978

Open
maxime-didier opened this issue Mar 7, 2024 · 6 comments
Open

Comments

@maxime-didier
Copy link

The problem

We are serving static files from a nix store path using wai-app-static, which relies on responseFile by default to send the contents over. The way this works, deployments are immutable folders in which every file has a timestamp of 1 (1970-01-01T00:00:01Z), an update is made by either updating a symlink somewhere or updating which nix store path is being served.

We have some custom logic to derive ETags from nix store paths, and wai-app-static handles If-None-Match headers accordingly. But this does not work because warp's handling of a responseFile without a specified FilePart relies on filesystem modification dates (Last-Updated + If-Modified-Since headers) and mistakenly overrides wai-app-static's cache handling.

The end result is that warp incorrectly sends 304 responses even though the file has actually changed and has a different ETag associated to it. This results in stale content being used from the browser's cache.

We have looked at warp's configuration options and have not found a way to disable this behavior.

Our workaround

At first, we instructed our internal users to issue a Ctrl+F5 to force their browsers to refresh their content cache. We've since setup our reverse proxy to strip the Last-Updated and If-Modified-Since headers from responses and requests respectively for the impacted services. This works pretty well and hasn't caused other issues so far.

Long term solution

I can see a few ways this issue could be solved. One would be to have warp check if ETags headers are present and avoid relying on modification times in such cases. Another would be to add a configuration option to make it possible to disable and/or customize how warp deals with filesystem modification times. I do not have strong beliefs on how this issue should be handled, so suggestions are welcome.

Once an approach has been decided, we are willing to submit a PR implementing it as soon as we are able to.

@maxime-didier maxime-didier closed this as not planned Won't fix, can't repro, duplicate, stale Mar 7, 2024
@maxime-didier maxime-didier reopened this Mar 7, 2024
@Vlix
Copy link
Contributor

Vlix commented May 21, 2024

I've tried to find which parts of the code you're having trouble with, but I'm still not sure what the issue is.

Could you mention files/lines (maybe copy paste sections of code) to more specifically point to the parts that are the cause and what the issues are per section?

@maxime-didier
Copy link
Author

The relevant wai-app-static line is here. There is a relevant comment in the warp implementation here, and the code is there.

@Vlix
Copy link
Contributor

Vlix commented Oct 19, 2024

@maxime-didier
Do I understand correctly that the issue is that warp ALWAYS adds a Last-Modified header when sending a "file without a FilePart"?
Because if the requester doesn't send an If-Modified-Since, it should work as expected. And since you're using ETags, I'd expect the requester to use If-Match or If-None-Match.

But I guess the requester is not under your control? (Like a browser?) And if warp sends a Last-Modified, then it will use If-Modified-Since?

@maxime-didier
Copy link
Author

This is indeed part of the issue. Our custom logic adds an ETag header, and warp adds a Last-Modified. The client receives both of these headers and this puts us in this unfortunate situation.

There's another moving part to this issue, though. A browser that received both headers will implement caching by sending both an If-None-Match and an If-Modified-Since and will continue to do so until the cache entry expires or it receives an updated resource. This means that removing the Last-Modified header from initial responses will not be enough to fix the problem if warp still preempts custom caching logic if it sees an If-Modified-Since. This is not a trivial issue because IIRC, the default cache expiration time is set to a far date into the future.

The core problem is that warp assumes it can rely on file modification times to implement caching. The Nix store is one example where this assumption fails, there may be others, ie. a filesystem that does not implement modification times.

@Vlix
Copy link
Contributor

Vlix commented Oct 21, 2024

Ok, I'm trying to find the spot where the override happens.

If both the If-None-Match and If-Modified-Since are given, it should produce a regular OK 200 (at least in the current code)

EDIT: This has been the case since warp-3.3.30

@Vlix
Copy link
Contributor

Vlix commented Oct 29, 2024

@maxime-didier Do you know version you are/were using? And if it was one before warp-3.3.30, could you try a newer version?

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

No branches or pull requests

2 participants