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

Honor Accept header when downloading directories #41037

Closed
maxnoe opened this issue Oct 21, 2023 · 4 comments · Fixed by #48098
Closed

Honor Accept header when downloading directories #41037

maxnoe opened this issue Oct 21, 2023 · 4 comments · Fixed by #48098

Comments

@maxnoe
Copy link

maxnoe commented Oct 21, 2023

How to use GitHub

  • Please use the 👍 reaction to show that you are interested into the same feature.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Is your feature request related to a problem? Please describe.

The file format one gets when downloading a directory seems to depend on the HTTP User-Agent and does not honor the Accept header.

macos user agents seem to get tar files, while everyone else gets zip files.

This makes it hard to write instructions to e.g. download files and unpack them via the command line (e.g. curl / wget) that are valid cross platform.

Describe the solution you'd like

It would be great if Nextcloud would honor the Accept header when downloading a directory, so that e.g. it is guaranteed that:

$ curl -LO -H 'Accept: application/x-tar' <share-link>
# or
$ curl -LO -H 'Accept: application/zip' <share-link>

always produces a tar file and a zip file respectively, independent of platform / user agent.

Describe alternatives you've considered

A possible alternative would be a query parameter for the filetype.

@maxnoe maxnoe added 0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement labels Oct 21, 2023
@solracsf
Copy link
Member

Yes.

// array of regexp. Matching user agents will get tar instead of zip
private array $preferTarFor = [ '/macintosh|mac os x/i' ];

@joshtrichards
Copy link
Member

You could always specify the User-Agent too. ;-)

Joking aside, unfortunately even User-Agent wouldn't be sufficient as archive size is also a factor:

if ($size > 0 && $size < 4 * 1000 * 1000 * 1000 && $numberOfFiles < 65536) {
$this->streamerInstance = new ZipStreamer(['zip64' => false]);
} elseif ($request->isUserAgent($this->preferTarFor)) {
$this->streamerInstance = new TarStreamer();
} else {
$this->streamerInstance = new ZipStreamer(['zip64' => PHP_INT_SIZE !== 4]);
}

Outside of possible quirks in the zip stream library we use (and maybe 32-bit support?), I wonder if there's still a need for different cross-platform archive handling?

@solracsf
Copy link
Member

solracsf commented Oct 23, 2023

I believe this was implemented because TAR is native on MacOS, and has (had?) some problems with Zip64 files.

@skjnldsv skjnldsv added 2. developing Work in progress and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Sep 17, 2024
@skjnldsv
Copy link
Member

Fixed by #48098

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants