The download archive should be named after the name of the current folder #22836

Closed
oparoz opened this Issue Mar 3, 2016 · 22 comments

Projects

None yet

8 participants

@oparoz
Contributor
oparoz commented Mar 3, 2016

Steps to reproduce

  1. Create a folder named Brazil_2016 which contains a sub-folder named Beaches
  2. Share the folder via link
  3. Open the link
  4. Download the folder by using the download button in the header
  5. Go into the sub-folder
  6. Click on "Download" again

Expected behaviour

The first download should be named Brazil_2016
The 2nd download should be named Beaches

Actual behaviour

The first download is named download
The 2nd download is named download(1)

It's a poor user experience.
The download endpoint used to support naming the archive using the arguments sent along the GET request, but that doesn't work any more.


@jancborchardt @rullzer @PVince81 @MorrisJobke

@jancborchardt jancborchardt added bug and removed enhancement labels Mar 3, 2016
@jancborchardt jancborchardt added this to the 9.1-next milestone Mar 3, 2016
@jancborchardt
Member

Yeah, this should definitely be fixed. As far as I remember this worked some time in the past already?

@oparoz
Contributor
oparoz commented Mar 3, 2016

That was also my impression, at least in some cases where it was possible to download content.
I've checked with 8.2.2 and it's the same problem on the Files side. On the Gallery side, you get a name only if you're on a sub-folder. IIRC with earlier version of Gallery+ you would get the folder name all the time, but something did change in core which broke the behaviour.

@PVince81
Collaborator
PVince81 commented Mar 4, 2016

Indeed, that looks fishy

@PVince81 PVince81 self-assigned this Mar 7, 2016
@PVince81
Collaborator
PVince81 commented Mar 7, 2016

Fix is here #22902

@oparoz
Contributor
oparoz commented Mar 7, 2016

Thanks for the quick fix!

@oparoz
Contributor
oparoz commented Mar 7, 2016

Re-opening as the fix is incomplete.

  1. Share a folder containing a folder names "superéàéàélongfolder name because i can"
  2. On the public side, descend into that folder
  3. Press the download button

The zip will be named: "super%C3%A9%C3%A0%C3%A9%C3%A0%C3%A9longfolder%20name%20because%20i%20can.zip"

That's unreadable for a "normal" user.

@PVince81 @MorrisJobke @jancborchardt

@oparoz oparoz reopened this Mar 7, 2016
@PVince81
Collaborator
PVince81 commented Mar 7, 2016

It's an issue with the zip/tar libraries: #19862, fixed in OC 9.0

Are you still seeing this in that version ?

@oparoz
Contributor
oparoz commented Mar 7, 2016

Yep, I've performed the test above on stable9.

@oparoz
Contributor
oparoz commented Mar 7, 2016

Response header in Firefox:

Content-Disposition:"attachment; filename*=UTF-8''super%25C3%25A9%25C3%25A0%25C3%25A9%25C3%25A0%25C3%25A9longfolder%2520name%2520because%2520i%2520can.zip; filename="super%25C3%25A9%25C3%25A0%25C3%25A9%25C3%25A0%25C3%25A9longfolder%2520name%2520because%2520i%2520can.zip""

(Same problem on Chrome), all on Windows

@oparoz
Contributor
oparoz commented Mar 7, 2016

Maybe #22489 broke it...

@PVince81
Collaborator
PVince81 commented Mar 7, 2016
@LukasReschke
Member

No idea. Works here on Linux and Mac. Don't have Windows 🙈

2016-03-07_19-15-12

@LukasReschke
Member

Aha. The ZIP it is. Let me test that as well :)

@PVince81
Collaborator
PVince81 commented Mar 7, 2016

Doesn't work on Linux with Chromium 48 either:
scheisse

Looks like it's time for bisecting.

@rullzer
Contributor
rullzer commented Mar 7, 2016

All those weird non-ascii languages! 😉

@LukasReschke
Member

Looks like it's time for bisecting.

Won't help. That never worked for you then if it really is #22836 (comment). Mind testing that locally ?

@PVince81
Collaborator
PVince81 commented Mar 7, 2016

@LukasReschke yeah, discarding those lines makes it work for me in Chromium and Firefox.

Can you make a PR ?

@LukasReschke
Member

Can you make a PR ?

Sure :)

@LukasReschke LukasReschke added a commit that referenced this issue Mar 7, 2016
@LukasReschke LukasReschke Remove double URL encoding
ZipStreamer as bundled with 9.0 will properly encode the filename already.

Fixes #22836 (comment)
1fd76b6
@LukasReschke
Member
@LukasReschke LukasReschke added a commit that referenced this issue Mar 7, 2016
@LukasReschke LukasReschke Remove double URL encoding
ZipStreamer as bundled with 9.0 will properly encode the filename already.

Fixes #22836 (comment)
fab42c7
@LukasReschke
Member

Stable9 at #22919

@oparoz
Contributor
oparoz commented Mar 7, 2016

Good job, thanks!, and that also takes care of the Gallery side without any additional changes :)

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