Imageprocessor: Cache header bug with storing images outside of web root

[nul800sebastiaan]

ImageProcessor is returning a 200 for each request to a processed image when the image is outside of the web root. After the first load of that image it should have returned in to a 304 - not modified for each consecutive request to the image.

When I change the cache location back to use App_Data/cache then it returns 304 after the first request as it should.

Luckily it doesn't seem to re-process the image and now updated or new cache file is created, but it does lead to more requests to the server as the images are not cached in the browser any more.

Not sure if this immediately rings a bell but if you have pointers on where to look I can help and create a PR for this.

[dlemstra]

I have not worked on ImageProcessor.Web yet but I did a quick look inside the code. I am not sure if this is the best solution but it looks like there should be a check if the "If-Modified-Since" header is set somewhere around here: https://github.com/JimBobSquarePants/ImageProcessor/blob/57b02304e60563291e7b214460763c8b5017ecb3/src/ImageProcessor.Web/HttpModules/ImageProcessingModule.cs#L541. The else of this if should check the value of the "If-Modified-Since" header and return a 304 if the image is not modified. You will need to compare that date with the date of the cache file. And the content inside the 'if' prob also deserves to be moved to a new method.

[JimBobSquarePants]

@dlemstra Bang on the money! 😄

Yeah @nul800sebastiaan stick an else on that and make sure the header is set and it should all be fine. It's due to us using TransmitFile instead of the StaticFileHandler outside the webapp.

A PR would be lovely.

[alindgren]

I am not sure if this should be considered part of this issue, or it's own, but I've noticed with the VirtualCachePath set outside of the web root, the headers look like this:

Cache-Control: private
Content-Encoding: gzip
Content-Type: text/html
Date: Fri, 13 Jan 2017 16:59:35 GMT
Server: Microsoft-IIS/10.0
Transfer-Encoding: chunked
Vary: Accept-Encoding

And when I set it back to store the cache in App_Data, the response headers look like this::

Accept-Ranges: bytes
Cache-Control: public, must-revalidate, max-age=31536000
Date: Fri, 13 Jan 2017 17:13:22 GMT
Etag: "7c42fc3ec06dd21:0"
Expires: Sat, 13 Jan 2018 17:13:23 GMT
ImageProcessedBy: ImageProcessor/2.5.0.0 - ImageProcessor.Web/4.7.0.0
Vary: Accept-Encoding

My experience is contrary to what is stated in the code comment at https://github.com/JimBobSquarePants/ImageProcessor/blob/develop/src/ImageProcessor.Web/Caching/DiskCache.cs#L269-L271 which states that the headers are the same.

[JimBobSquarePants]

Thanks for that info @alindgren I'll do some testing.

[alindgren]

I've testing out some changes and I think I will have a PR soon if you want.

[JimBobSquarePants]

Yes please! 👍

[JimBobSquarePants]

I've just pushed the fix @alindgren wrote for this but I can't ship yet until I have a look at #539