Open Bug 1575799 Opened 2 months ago Updated Last hour

Download from a synology NAS is failing

Categories

(Core :: DOM: Core & HTML, defect, P1)

defect

Tracking

()

Webcompat Priority ?
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- affected
firefox67 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- wontfix
firefox71 --- affected

People

(Reporter: dany_trafic, Unassigned)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [regression][necko-triaged])

Attachments

(2 files)

Attached image Firefox.PNG

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

i'm triyng to download multiple files from Synology NAS File Station.
This is happening for all Firefox version above 66.0.5, also for developer versions 69 and 70 as tested now 22 of august 2019.

Actual results:

When i try to download multiple files from My Synology NAS File Station i get this error:

could not be saved, because the source file could not be read.
Try again later, or contact the server administrator.

Tried on several windows computer, diffrent Firefox installs, and diffrent Synology disk station's.

Expected results:

Download multiple files as one .zip

Daniel, why do you think it isn't a problem with the Synology software?

Flags: needinfo?(dany_trafic)
Summary: synology → Download from a synology NAS is failig

Hello Sylvestre Ledru, I incline to think it’s a Firefox problem mainly because on every other browser it’s working as it should, no errors, just downloading files, on top of that with previous Firefox builds (66.0.5 and before)it works like a charm, so maybe I’m missing something? Also I tested this on several different computers, and also on 6 different Synology NAS File Station with the same results.

Flags: needinfo?(dany_trafic)

Hi Daniel,

Does this issue occur with a fresh profile? you can find the steps here: https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles?redirectlocale=en-US&redirectslug=Managing-profiles#w_starting-the-profile-manager

Can you please download Firefox Nightly from here: https://nightly.mozilla.org/ and retest the problem and see if the issue still occurs there as well?

If after doing this you can still reproduce the bug, could you send me a screenshot or a video of the bug? That always helps a lot.

Thanks in advance, Flor.

Flags: needinfo?(dany_trafic)

Hi Daniel,

I'm marking this as Resolved-Incomplete due to lack of response. If the issue is still reproducible with the latest Firefox version, feel free to reopen the bug with more information.

Best regards, Flor.

Status: UNCONFIRMED → RESOLVED
Closed: 14 days ago
Resolution: --- → INCOMPLETE

Hello @Florencia,

Sorry for the late reply, (i was on holidays), I tried on Firefox nightly, same error, it doesn't matter if i use a fresh profile or not the error is the same, this can be reproduced at any moment, like i said before, i tried this on several machines, some with a old profile, some with new profile/fresh install (windows + browser). This is not an intermitent bug, it's a permanent one that can be reproduced at any moment!

Flags: needinfo?(dany_trafic)

Hi Daniel,

I've chosen a component for this bug so that the developers may look at it. We'll await their answer. If you consider that there's another component that's more proper for this case you may change it.

Regards, Flor.

Status: RESOLVED → UNCONFIRMED
Component: Untriaged → Networking: File
Product: Firefox → Core
Resolution: INCOMPLETE → ---
Version: 68 Branch → Trunk
Assignee: nobody → juhsu
Priority: -- → P1
Summary: Download from a synology NAS is failig → Download from a synology NAS is failing
Whiteboard: [regression][necko-triaged]

Hello Daniel,
Since it works for previous Firefox builds (66.0.5 and before).
It would be very helpful if you could find the exact regression range.
https://mozilla.github.io/mozregression/quickstart.html

Could you help us?
Thanks!

Flags: needinfo?(dany_trafic)

I did the mozregression

2019-10-01T15:40:45: DEBUG : Starting merge handling...
2019-10-01T15:40:45: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=a01586b62cf510bb165057e0bea9a45cc76e961e&full=1
2019-10-01T15:40:45: DEBUG : Found commit message:
Bug 1489308 part 10.  Remove some document.open handling in outer window that's no longer needed.  r=mccr8

Differential Revision: https://phabricator.services.mozilla.com/D18077
Assignee: juhsu → nobody
Component: Networking: File → DOM: Core & HTML
Flags: needinfo?(dany_trafic)

(In reply to Junior [:junior] from comment #9)

I did the mozregression

2019-10-01T15:40:45: DEBUG : Starting merge handling...
2019-10-01T15:40:45: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=a01586b62cf510bb165057e0bea9a45cc76e961e&full=1
2019-10-01T15:40:45: DEBUG : Found commit message:
Bug 1489308 part 10.  Remove some document.open handling in outer window that's no longer needed.  r=mccr8

Differential Revision: https://phabricator.services.mozilla.com/D18077

Hey Junior,

I also did the Mozregression-with the same results:

2019-10-02T14:09:05: DEBUG : Starting merge handling...
2019-10-02T14:09:05: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=a01586b62cf510bb165057e0bea9a45cc76e961e&full=1
2019-10-02T14:09:06: DEBUG : Found commit message:
Bug 1489308 part 10. Remove some document.open handling in outer window that's no longer needed. r=mccr8
Differential Revision: https://phabricator.services.mozilla.com/D18077
2019-10-02T14:09:06: DEBUG : Did not find a branch, checking all integration branches
2019-10-02T14:09:06: INFO : The bisection is done.
2019-10-02T14:09:07: INFO : Stopped

So...does that help finding the problem/bug?

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression

Hi Boris, mozregression is pointing at bug 1489308 as the regressor for this issue. Can you please take a look?

I would really like to see the scripts involved. It sounds like the page is calling document.open() and that's canceling the downloads involved, but the details of why it's not happening in other browsers and didn't use to happen in Firefox (since document.open used to cancel things before my changes too, does it in all browsers, and is required to do it per spec) really depend on the exact script.

If someone has a setup that shows that bug that I can access, that would obviously be ideal, along with specific steps to reproduce. If not, at least copies of the scripts and HTML involved would be very helpful.

Flags: needinfo?(juhsu)
Flags: needinfo?(dany_trafic)
Flags: needinfo?(bzbarsky)

I've pinged my friend who works for Synology. This will be fixed in their next release.

Here's the reason I understood:
iframe.contentDocument.open/write/close three calls in one shot.
An unexpected load event triggers their error handling process.

Maybe Bug 1489308 forced the load event even if the close is called or maybe changed some event timing .
Does it make sense?

Flags: needinfo?(juhsu) → needinfo?(bzbarsky)

Thank you for checking on that! There were some changes around the load event there, and there are some differences around the details from Chrome, and the spec is useless. See https://github.com/whatwg/html/issues/4291 and https://github.com/whatwg/html/issues/4292

I would still really like to know what their exact script looks like so it can feed back into our behavior and maybe the spec issues. Are they basically doing open/write/close and adding a load event listener after that, and in Chrome the load event has already fired (under close()) while we fire it async? Or something else?

Flags: needinfo?(bzbarsky) → needinfo?(juhsu)

Here's the code snippet:

var iframe = document.createElement('iframe');
document.body.appendChild(iframe);
var fn = () => {
	iframe.remove();
	console.log('error');
};
var doc = iframe.contentDocument;
doc.open();
doc.close();
iframe.addEventListener('load', fn, true);
iframe.src = "https://www.seattlesymphony.org/~/media/files/notes/schubert-sonata-21-b-flat.pdf";

iframe.src can be replaced with any Content-Disposition: attachment

Flags: needinfo?(juhsu) → needinfo?(bzbarsky)

IMO comment 16 is informative enough for :bz. Feel free to give more information.

Flags: needinfo?(dany_trafic)

Thank you. What's going on there is exactly my guess from comment 15. If you make the testcase look like so:

   var iframe = document.createElement('iframe');
   document.body.appendChild(iframe);
   iframe.addEventListener('load', () => {
     console.log('load fired');
   });

   var doc = iframe.contentDocument;
   doc.open();
   console.log("About to call close");
   doc.close();
   console.log("Called close");

you see that in Firefox the load event fires after the close() call returns (and hence after they add the load listener in the snippet in comment 16), but in Chrome it fires before close() returns.

That said, the behavior of my testcase in this comment is the same before the bug 1489308 changes as well: the load event fires after doc.close() returns. So the difference is that if a navigation is started after the document.close() call but before the load event fires, then the load event never happens at all before bug 1489308. That's a consequence of the document.open bits looking like an (unfinished) navigation when the src set happens before bug 1489308, and the src set canceling the navigation and hence causing it to never fire a load event...

Per spec, our behavior before bug 1489308 is definitely wrong. Or course per spec Chrome's behavior is also wrong: see https://bugs.chromium.org/p/chromium/issues/detail?id=878491

Maybe we could add a special case for firing the load event immediately during close if there was an open with no write after it, and get the spec changed accordingly somehow....

What I don't understand is why the page is doing the open/close dance here at all. Just taking all that out and making the script be like so:

var iframe = document.createElement('iframe');
document.body.appendChild(iframe);
var fn = () => {
	iframe.remove();
	console.log('error');
};
iframe.addEventListener('load', fn, true);
iframe.src = "https://www.seattlesymphony.org/~/media/files/notes/schubert-sonata-21-b-flat.pdf";

seems to work in browsers, though it also works only due to browser bugs. In particular, it only works in Chrome because it fires the load event sync under the append instead of async like https://html.spec.whatwg.org/multipage/iframe-embed-object.html#process-the-iframe-attributes says to do, and it only works in Firefox because the navigation starting cancels the (non-spec; see bug 1447406) about:blank load that would have fired the load event!

The spec-compliant way to do this is:

var iframe = document.createElement('iframe');
iframe.addEventListener("load", () => {
    var fn = () => {
      iframe.remove();
      console.log('error');
    };
    iframe.addEventListener('load', fn, true);
    iframe.src = "https://www.seattlesymphony.org/~/media/files/notes/schubert-sonata-21-b-flat.pdf";
  }, { once: true });
document.body.appendChild(iframe);

which is rather annoyingly complicated... :(

Flags: needinfo?(bzbarsky)

Fwiw, I filed https://github.com/whatwg/html/issues/4965 on some of the spec issues here.

Junior, do you know when Synology's next release is? Do they generally get it rolled out to all their installs?

That is, do we need to do something on our end to either get this fixed for people sooner or to make sure all our users who also use Synology get the fix?

Flags: needinfo?(juhsu)

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #20)

Junior, do you know when Synology's next release is? Do they generally get it rolled out to all their installs?

That is, do we need to do something on our end to either get this fixed for people sooner or to make sure all our users who also use Synology get the fix?

It's hard to say. Last release is Apr.
Next release, which is a dot release, should be Oct or Nov.
The default update policy is automatically update. I use the default as well.
They do have business users. I guess they could use ESR 60.x but it's going to end of life.
Given low risk, they should feel safe to upgrade.

What I don't understand is why the page is doing the open/close dance here at all.

In fact, they use multiple open/close pairs. The first one is to clear the doc, and the second is to write something from a clear plate.
The fix is simple, using dom operation directly instead of open/close trick.

The code snippet below shows the intension.
Hope it helps. Thanks for taking care of it and giving me a DOM lecture :) Really enjoy.

var iframe;

function prepareIframe()
{
	iframe = document.createElement('iframe');
	document.body.appendChild(iframe);
};

function clickButtonEvent() {
	var doc = iframe.contentDocument;

	var cleanDocContent = function() {
		doc.open();
		doc.close();
	};	
	var writeDocContect = function() {
		//Pass sensitive and too long parameters in HTTP body
		var html = '<html>' +
					'<head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head>' +
					'<body>' +
						'<form accept-charset="utf-8" name="form" action="https://www.seattlesymphony.org/~/media/files/notes/schubert-sonata-21-b-flat.pdf" method="POST">' +
							'<input type="hidden" name="password" value="**" />' +
							'<input type="hidden" name="path" value="/volume1/1,/volume1/2" />' +
						'</form>' +
				'</body></html>';
		doc.open('text/html');
		doc.write(html);
		doc.close();
	};

	cleanDocContent();
	writeDocContect();
	var count = 1;
	iframe.addEventListener('load', function() {
                // remove iframe triggers download error
		//iframe.remove();
		console.log('Load event times:', count);
		++count;
	}, { once: false });

	doc.form.submit();
};

prepareIframe();
setTimeout(clickButtonEvent, 2000);
Flags: needinfo?(juhsu)

Next release, which is a dot release, should be Oct or Nov.

OK. We'd be unlikely to ship anything here before Firefox 71 ships in December, so sounds like they will get to this first.

The first one is to clear the doc, and the second is to write something from a clear plate.

Ah, I see. And they're relying on the fact that none of the content they write involved anything loading, because if they did their code would break in every browser (since the load event would have to fire async from close() if there were subresources)... Alright. That makes me worry a little less about someone else also being affected by this.

In particular, the "close after open with no write should fire load sync" approach would not help in their case, since they are writing out content...

Oh, and even if they switch to DOM insertions, if we aligned with the spec by fixing bug 1447406 their code would still break. The spec on that may change, though; see https://github.com/whatwg/html/issues/4965

Status: UNCONFIRMED → NEW
Ever confirmed: true
Webcompat Priority: --- → ?
You need to log in before you can comment on or make changes to this bug.