WUI: the "Download!" button on the Welcome page leads to the file displayed in-browser instead of downloaded/saved

[zooko]

I just had a call from a customer ("TV") who was surprised that the "Download!" button on the WUI caused the file to display in the web browser (this file was of text/plain type) instead of downloading/saving. I was surprised, too. Investigating, I see that while we've long had a force-download feature​, the current WUI doesn't use it!

I reproduced the customer's issue with Firebug turned on and observed the following HTTP transactions.

1. When I click "Download!" my browser issues:

GET /uri?uri=$CAP&filename=testdown3.txt

and the tahoe-lafs WUI/WAPI sends a 302 Found status:

HTTP/1.1 302 Found
Location: http://127.0.0.1/uri/$CAP?filename=testdown3.txt

and then the browser issues:

GET /uri/$CAP?filename=testdown3.txt

After which the server sends the contents of the text file, along with header Content-Type: text/plain and no Content-Disposition header, and so the browser displays the contents of the file in the browser window.

To close this ticket, make it so that the last request issued by the browser uses the third form for Viewing/Downloading a File​, i.e. instead of /uri/$CAP?filename=testdown3.txt, it should request /named/$CAP/filename=testdown3.txt?save=true. I just tested and confirmed that this makes the server include the Content-Disposition header, but /named/$CAP/filename=testdown3.txt?save=true does not.

[davidsarah]

I don't think it's correct that the WUI never uses ?save=true. It is used on the Upload results page, as stated in #1551.

My preference would be to include both Download and View links/buttons:

• in each entry of a WUI directory listing (which is #827)
• on the Upload results page (which is #1551)
• in the "Download a file" form of the Welcome page (this ticket).

[davidsarah]

Oh, the Welcome page does already have a View! button. But it doesn't have a filename field in the "View a file or directory" form. I kind-of see how that makes sense, especially for directories, but it results in the browser doing content-sniffing without any information about the type of a file, which I don't like.

[davidsarah]

Replying to davidsarah:

I don't think it's correct that the WUI never uses ?save=true. It is used on the Upload results page, as stated in #1551.

Hmm, looking at the source implementing that link (render_download_link in web/unlinked.py), I was mistaken in thinking it uses ?save-true.

[warner]

User "lcstyle" on IRC noticed this same problem tonight, and might have time to hack on it. I suggested the following:

• src/allmydata/web/root.py Root.render_download_form() (line 304) constructs the welcome page's download form
• that form uses method="GET" and sends the browser to a URL like /uri?uri=FILECAP&filename=FILENAME
• src/allmydata/web/root.py URIHandler.render_GET() (line 28) is responsible for handling that GET, and currently responds by issuing a 302 Redirect to /uri/FILECAP?filename=FILENAME
• we really want it to redirect to /named/FILECAP/FILENAME?save=true

We don't want to change the redirect behavior for existing URLs, since it's documented to retrieve the contents (and *not* add the download-only Content-Disposition header). So I think we should make render_download_form() add a third queryarg (via an HTML hidden input field), "download=true". When URIHandler.render_GET() sees download=true, it should use the different redirect behavior. Otherwise it should do the old behavior.

According to the docs, using /uri/FILECAP?filename=FILENAME&save=true is supposed to result in a download. I'm not sure if this could possibly get the filename right (which I think is why we added the /named/FILECAP/FILENAME form, so that browsers would see a "filename" in the place they expect it). We should test that and maybe update the docs (docs/frontends/webapi.rst, line 1048, "Viewing/Downloading a File") to remove that claim.

[Lcstyle]

I am looking to address two issues here:

• we want to force all files to download.
• the filename field for the download form is ignored.

[Lcstyle]

Pull Request for fix submitted:

​https://github.com/tahoe-lafs/tahoe-lafs/pull/112

[daira]

Fixed by [62a0ec0820ae9966d60528ee45a356f5161ee839/trunk]. I judged it was not necessary to have a test, because absent a way of simulating what a browser would do, I can't see a way to test the actual functionality. (It would be possible to test that the form field is present, but I don't think that tests that just duplicate what is obviously in the .xhtml template are particularly valuable.)