[MGNLUI-3274] Make sure DownloadBinaryAction properly propagates Content-Disposition to Vaadin Servlet Created: 16/Sep/14  Updated: 06/Aug/15  Resolved: 01/Dec/14

Status: Closed
Project: Magnolia UI
Component/s: None
Affects Version/s: 5.3.5
Fix Version/s: 5.3.6

Type: Bug Priority: Major
Reporter: Lars Fischer Assignee: Christoph Meier
Resolution: Fixed Votes: 0
Labels: assets, support, usability, uzh
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relation
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

The "download asset" funtion (depending on the MIME type) shows the asset in the current browser window, this forces the user to unexpectedly leave the application.

Example: Firefox with an MP4 Asset using a Mac.

The system dialog to save the asset should appear independently of the MIME type of the asset to download.



 Comments   
Comment by Christoph Meier [ 21/Nov/14 ]

Situation before the fix:

The action which initiates the download is DownloadBinaryAction. There
Page.getCurrent().open(streamResource, null, false);
is called .
com.vaadin.server.Page#open(Resource resource, String windowName, boolean tryToOpenAsPopup)
is deprecated ... so, would be nice to get rid of the deprecated call here.

The request is handled by AdminCentralVaadinServlet#service which calls super.service() which delegates to VaadinServlet.
After a few other "delegations" finally the stream is written on com.vaadin.server.DownloadStream#writeResponse.
=> There the "Content-Disposition" header is set.

DownloadStream has the method #setParameter which allows to set parameters which then will be added as response-headers. This is also done on DownloadBinaryAction (see line 107).

resource.getStream().setParameter("Content-Disposition", String.format("attachment; filename=\"%s\"", fileName));

However, the DownloadStream on which the parameter is set in DownloadBinaryAction is NOT the same instance as it is used by VaadinServlet/VaadinService; GlobalResourceHandler instantiates another DownloadStream (see line 111).
=> at the end DownloadStream has not set a paramter with the key "Content-Disposition" and DownloadStream#writeResponse sets "Content-Disposition" and it does it in a way which is not properly interpreted by Firefox (see DownloadStream line 286 ff.).

String contentDispositionValue = getParameter("Content-Disposition");
  if (contentDispositionValue == null) {
    contentDispositionValue = "filename=\"" + getFileName() + "\"";
    response.setHeader("Content-Disposition", contentDispositionValue);
  }

"Conclusion":
For me it looks like, the "bug" is in the Vaadin classes which instantiates the DownloadStream 2 times and finally in DownloadStream which writes the "Content-Disposition" response-header in a way which is not correctly interpreted by Firefox.

Possible "solutions":

  • Find a way that Vaadin does use the same DownloadStream as it was instantiated by DownloadBinaryAction (but i have no idea how to do that).
  • Use another - not deprecated - method com.vaadin.server.Page#open() within DownloadBinaryAction where not StreamSource but a String (an uri) is passed. The URI is used as requestURI and it could be constructed in a way that it is mapped to a Magnolia servlet; but it should be a Servlet form UI-project (and such servlet in ui is not yet existing).
  • Patch com.vaadin.server.DownloadStream#writeResponse
Comment by Christoph Meier [ 25/Nov/14 ]

A solution has been found to make sure the DownloadStream used in the VaadinServlet/VaadinService is the same like the one created in DownloadBinaryActionTest which makes sure the Content-Disposition can be passed as parameter and is set in a form which is also understood by Firefox.

The fix is on branch "MGNLUI-3274_5.3.x".

Since DownloadBinaryAction is only used by DAM app, we should add a dedicated DownloadAssetAction to dam which will also allow to use the DamDownloadServlet (see follow-up ticket MGNLDAM-524).

Comment by Christoph Meier [ 01/Dec/14 ]

Created subclass of StreamResource - info.magnolia.ui.vaadin.server.DownloadStreamResource - which contains the workaround (overriding StreamResource#getStream) for better testability and usability.

Comment by Christoph Meier [ 08/Dec/14 ]

Integrated into master and magnolia-ui-5.3.x.

Comment by Peili Liang [ 09/Dec/14 ]

I use Firefox(version:24.8.1) to download a mp4 file, it is ok!

Generated at Mon Feb 12 09:04:57 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.