search

gildas-lormeau / zip.js

JavaScript library to zip and unzip files supporting multi-core compression, compression streams, zip64, split files and encryption.
https://gildas-lormeau.github.io/zip.js
BSD 3-Clause "New" or "Revised" License
3.33k stars 506 forks source link

Range request optimalizations #498

Closed bwbroersma closed 3 months ago

bwbroersma commented 3 months ago

There is an option to 'force' range requests.

  1. In case of CORS, maybe it's also nice to hint that Access-Control-Expose-Headers: Content-Range is not supported, so a HEAD request is done, saving one request with Range: bytes=0-0 where the Content-Range cannot be accessed.
  2. One can also do a suffix range request: Range: bytes=-22, however this will need a preflight if CORS, since rangeValue[0] is null is excluded from the 'CORS-safelisted request-header'. In case of CORS this will ask for a Access-Control-Request-Headers: range in the preflight, want needs to be answered with Access-Control-Allow-Headers: range. I've seen servers who will respond to this suffix range giving the complete content (so no range at all).

These two hints both could remove one unnecessary request, reading a directory can be reduced to 2 requests on the same server and 3 if it's CORS (either one preflight + 2 requests, or 3 simple requests, which could be 3×GET or HEAD+2×GET). If it's on the same server, 2 can be done, if it's know it supports this (e.g. nginx). Probably it's the safest and most efficient to always go for HEAD if CORS, and have an explicit option to enable the combination of a fetch EOCD + size.

An explicit preventHeadRequest:false could do the first thing. This fix does not yet 'default' to HEAD on CORS. The second part is trickier, there are multiple options, add some special caching of the EOCD block, or make a larger refactor by making the size optional (but I'm afraid that's more complexity than my current EOCD cache hack).

gildas-lormeau commented 3 months ago

Thank you for this detailed description of the problem. The optimizations you suggest suit me too, and I have nothing to say about your pull requests. Looks perfect to me :)

gildas-lormeau commented 3 months ago

The new version 2.7.41 including your changes is now available for download.

bwbroersma commented 3 months ago

Thanks, in hindsight I could have used one PR. Note that I did add it to the typescript definition, but not to any other documentation, but apparently that's generated from the typescript definition :) Like mentioned in:

In case of preventHeadRequest: the default value false does not really capture the fact of the 'undefined or true' check: https://github.com/gildas-lormeau/zip.js/blob/7021dd3a036853fabd3dc733a30b7b823e87d5d4/lib/core/io.js#L327

gildas-lormeau commented 3 months ago

I can confirm that for the documentation, your additions are sufficient, I think.