File download affects active Ajax requests

RESOLVED INVALID

Status

()

Core
Networking
RESOLVED INVALID
a year ago
a year ago

People

(Reporter: Eric, Assigned: kershaw)

Tracking

({testcase})

52 Branch
x86_64
Windows 10
testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [necko-active] DUPEME)

Attachments

(2 attachments)

(Reporter)

Description

a year ago
Created attachment 8808261 [details]
Network tab showing 2 xhr POSTs having no status, 0 B, 0 ms following POST of "report?output=xl" document download.  8 rows above see same xhr POSTs with appropriate status and responses.

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36

Steps to reproduce:

While several active XHR requests are awaiting their responses, POST a form to a web service that responds with plain text containing the following headers:
  Response.AddHeader("Content-Description", "File Transfer")
  Response.AddHeader("Content-Disposition", "attachment; filename=" & str1 & ".xls")
  Response.AddHeader("Content-Transfer-Encoding", "binary")
  Response.AddHeader("Content-Type", "text/html")



Actual results:

One or more of the active XHR requests receive no response nor status return, or receive a truncated response (resulting in a JSON Parse.error), depending on the actual timing of when the web service downloading file contents sends its response.  The file download does complete successfully and the downloaded file's contents are correct.


Expected results:

The active XHR requests should receive their expected JSON responses without being truncated or completely eliminated, just as they do if the concurrent form POST to download a text file isn't requested by the end user.  

Please note that the expected results do occur properly when testing the same steps in both Google Chrome and MS Edge.
(Reporter)

Updated

a year ago
Keywords: dataloss, reproducible
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64

Comment 1

a year ago
Could you provide a simplified testcase to reproduce the issue, please.
Component: Untriaged → Networking
Flags: needinfo?(efriedm)
Keywords: dataloss, reproducible → testcase-wanted
Product: Firefox → Core
(Reporter)

Comment 2

a year ago
This xhr error occurs in a complex MVC system where the Viewer is written in JavaScript running on the browser, which issues numerous xhr calls to Controllers running in .NET 4.5 under IIS. It would require a significant programming effort to set up a simplistic case.
  
Instead, I’ve established login credentials for you on our Development server, and provided a short illustrated roadmap for you to follow to reliably trigger the problem.  I've sent that information in an separate email to Loic (to protect the security of the login credentials).

Comment 3

a year ago
Thanks, I'll check your email, and test to see if there is a possible regression in Firefox. If a Mozilla dev asks for credentials, I'll forward it to him too.
Whiteboard: [necko-backlog]
(Reporter)

Comment 4

a year ago
(In reply to Loic from comment #3)
> Thanks, I'll check your email, and test to see if there is a possible
> regression in Firefox. If a Mozilla dev asks for credentials, I'll forward
> it to him too.

Have you been successful using the login credentials I supplied to trigger the truncated/suppressed xhr response problem when simultaneously posting a form to download a plain text file with header "Content-Disposition", "attachment; filename=xxx"?

Comment 5

a year ago
I tested the testcase your provided by email, I'm able to reproduce the issue, with e10s on/off.
I check if it was a recent regression, that's not the case, Firefox 38 has the same behavior too.

Patrick, is there someone with time who can test and start to debug the testcase? The testcase is simple, only a few clicks on the page with the Netmonitor open (persistant lohs checked).
Flags: needinfo?(efriedm) → needinfo?(mcmanus)
Keywords: testcase-wanted → testcase
loic can you generate an http_log from nspr? (on nightly/devchannel you can do it dynamically from about:networking)

I suspect something is cancelling the whole load group - probably the download manager - and necko is just responding to it. in which case the toolkit/dlmgr is the component to look at - but we can tell from the log if there is a cancel()
Flags: needinfo?(mcmanus)

Comment 7

a year ago
Created attachment 8814290 [details]
log.txt-main.6276

2 XHR requests received no response in the log (I verified in the Netmonitor) just before downloading the Excel file.
(Reporter)

Comment 8

a year ago
While the "no response" situation is typical, I would like you to note that, depending on the actual timing of the Excel download request, occasionally a pending XHR request will start to receive its (JSON) response, but that response is then truncated by the Excel download request.  This typically results in an invalid JSON parse exception being thrown, and is how we noticed the problem.  Perhaps that will help shed some light on the cause of the problem?
Kershaw will take a look at this problem.  Could Eric or Loic forward him the simplified test case to reproduce it?  Thanks.
Assignee: nobody → kechang

Updated

a year ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Updated

a year ago
Whiteboard: [necko-backlog] → [necko-active]

Updated

a year ago
Flags: needinfo?(epinal99-bugzilla2)
Flags: needinfo?(efriedm)
(Reporter)

Comment 10

a year ago
I am happy to send Kershaw the simplified test case documentation but I'll need his email address.

Comment 11

a year ago
(In reply to Shian-Yow Wu [:swu] from comment #9)
> Kershaw will take a look at this problem.  Could Eric or Loic forward him
> the simplified test case to reproduce it?  Thanks.

I sent him the testcase by email.
Flags: needinfo?(epinal99-bugzilla2)
Flags: needinfo?(efriedm)
(Assignee)

Comment 12

a year ago
(In reply to Loic from comment #11)
> (In reply to Shian-Yow Wu [:swu] from comment #9)
> > Kershaw will take a look at this problem.  Could Eric or Loic forward him
> > the simplified test case to reproduce it?  Thanks.
> 
> I sent him the testcase by email.

Thanks. I am bale to reproduce this bug.

As Patrick said in comment #6, the whole load group is canceled. The call stack [2] shows that it is canceled by |nsDocShell::InternalLoad| [1].

However, I have no idea about the reason behind this. Perhaps bz knows a thing or two?

[1] http://searchfox.org/mozilla-central/rev/d98418da69edeb1f2f8e6f3840157fae1512f89b/docshell/base/nsDocShell.cpp#10592
[2] call stack
 frame #0: 0x00000001031f0efd XUL`mozilla::net::nsHttpChannel::Cancel(this=0x000000011b887000, status=2152398850) + 157 at nsHttpChannel.cpp:5574 [opt]
    frame #1: 0x00000001032019ad XUL`non-virtual thunk to mozilla::net::nsHttpChannel::Cancel(this=<unavailable>, status=<unavailable>) + 13 at nsHttpChannel.cpp:5565 [opt]
    frame #2: 0x0000000102ebadf1 XUL`mozilla::net::nsLoadGroup::Cancel(this=<unavailable>, status=<unavailable>) + 705 at nsLoadGroup.cpp:273 [opt]
    frame #3: 0x0000000103a1722f XUL`nsDocLoader::Stop(this=<unavailable>) + 351 at nsDocLoader.cpp:243 [opt]
    frame #4: 0x00000001062f934f XUL`nsDocShell::Stop(unsigned int) [inlined] nsDocShell::Stop(this=<unavailable>) + 543 at nsDocShell.h:189 [opt]
    frame #5: 0x00000001062f9347 XUL`nsDocShell::Stop(this=0x0000000121b58000, aStopFlags=1) + 535 at nsDocShell.cpp:5525 [opt]
    frame #6: 0x000000010630b5b3 XUL`nsDocShell::InternalLoad(this=<unavailable>, aURI=<unavailable>, aOriginalURI=0x0000000000000000, aLoadReplace=<unavailable>, aReferrer=<unavailable>, aReferrerPolicy=1, aTriggeringPrincipal=<unavailable>, aPrincipalToInherit=<unavailable>, aFlags=6, aWindowTarget=<unavailable>, aTypeHint=<unavailable>, aFileName=0x000000010930c050, aPostData=0x0000000000000004, aHeadersData=<unavailable>, aLoadType=<unavailable>, aSHEntry=<unavailable>, aFirstParty=<unavailable>, aSrcdoc=<unavailable>, aSourceDocShell=<unavailable>, aBaseURI=<unavailable>, aDocShell=<unavailable>, aRequest=<unavailable>) + 13395 at nsDocShell.cpp:10599 [opt]
    frame #7: 0x000000010633c961 XUL`nsDocShell::OnLinkClickSync(this=<unavailable>, aContent=<unavailable>, aURI=<unavailable>, aTargetSpec=<unavailable>, aFileName=0x000000010930c050, aPostDataStream=0x000000011b6ffb30, aHeadersDataStream=<unavailable>, aDocShell=<unavailable>, aRequest=<unavailable>) + 2769 at nsDocShell.cpp:14022 [opt]
    frame #8: 0x000000010500f79e XUL`mozilla::dom::HTMLFormElement::SubmitSubmission(this=0x0000000127411000, aFormSubmission=<unavailable>) + 1294 at HTMLFormElement.cpp:829 [opt]
    frame #9: 0x000000010500f016 XUL`mozilla::dom::HTMLFormElement::DoSubmit(this=0x0000000127411000, aEvent=<unavailable>) + 454 at HTMLFormElement.cpp:686 [opt]
    frame #10: 0x000000010500de3a XUL`mozilla::dom::HTMLFormElement::Submit(mozilla::ErrorResult&) [inlined] mozilla::dom::HTMLFormElement::DoSubmitOrReset(this=0x0000000127411000, aEvent=<unavailable>, aMessage=eFormSubmit) + 90 at HTMLFormElement.cpp:606 [opt]
    frame #11: 0x000000010500dde0 XUL`mozilla::dom::HTMLFormElement::Submit(this=0x0000000127411000, aRv=0x00007fff5fbfb8f8) + 48 at HTMLFormElement.cpp:262 [opt]
    frame #12: 0x0000000104bdf6fd XUL`mozilla::dom::HTMLFormElementBinding::submit(cx=0x0000000110719000, obj=<unavailable>, self=<unavailable>, args=0x00007fff5fbfb958) + 61 at HTMLFormElementBinding.cpp:661 [opt]
Flags: needinfo?(bzbarsky)
It's very long-standing behavior: all outstanding requests are canceled when a navigation _starts_ (which is before we know whether it will complete), because canceling them after you actually unload the page is not  web-compatible, and you _do_ generally want to cancel stuff when you navigate.  This behavior has been in place for at least 15 years, I believe.

Note that this behavior is also quite clearly specified in the HTML spec, for what it's worth, though there has been some discussion about trying to change that (e.g. by doing the canceling right before firing unload, and not canceling the actual load for the navigation or something).  If the spec gets changed, and there is some indication that the changed spec is web-compatible (e.g. if someone else ships it without the world blowing up in their face), we can talk about changing our behavior.

I think this is all discussed at length in other bugs, for what it's worth.
Flags: needinfo?(bzbarsky)
Whiteboard: [necko-active] → [necko-active] DUPEME
(Assignee)

Comment 14

a year ago
I think we can close this bug since this seems to be an expected behavior according to comment#13.

What do you think, Eric?
Flags: needinfo?(efriedm)
(Reporter)

Comment 15

a year ago
I can understand why navigation AWAY from a page that issued AJAX calls would cancel those calls, since the page that issued them has been/will be unloaded.  

However, in my considered opinion, Content-disposition: attachment is NOT navigation because it doesn't unload the current page; quite the contrary, it's a download of data while REMAINING on the current page.  It should not result in canceling of pending AJAX calls since the page that issued them remains the current page.  

This is conceptually no different than an AJAX request that supplies additional information to be displayed on the current page, which is also clearly not navigation.  How else would a persistent page download a file if not by submitting a form that returns the file contents using Content-disposition: attachment?

I should reiterate that Mozilla's behavior in canceling these AJAX calls also truncates responses that have already begun to be received.  This truncation of a JSON response results in a JSON parse error exception.  That's clearly a serious bug.

In thorough testing, neither Google Chrome nor MS Edge exhibit this same behavior problem.  Both of them properly treat Content-disposition: attachment as NOT being page navigation, because it doesn't unload the current page.
> However, in my considered opinion, Content-disposition: attachment is NOT navigation

Per spec it is navigation until you get the response; then the navigation is aborted.

> This is conceptually no different than an AJAX request

It's conceptually different because the AJAX request cannot possibly navigate, whereas a form submit _can_ and you don't know whether it _will_ until you get the server response.

Again, if you think the spec here should change, this discussion should go into a spec issue.  Maybe you'll have some luck getting the Google and Microsoft folks to talk to you and describe what they're actually doing.  I haven't had such luck.

In the meantime, as I said above we are doing exactly what the spec says to do.
(Reporter)

Comment 17

a year ago
Boris-  I thank you for your quick response.  

If we assume that your interpretation of the specs is correct, I would ask you to please describe how a client JavaScript implementation that makes AJAX calls to web services properly should request that a file be downloaded from the server so that AJAX transactions are not affected?

Without asking Google & Microsoft, it appears quite obvious that they are inspecting the response headers to determine whether or not the response is navigation _before_ canceling their AJAX calls.  Clearly that's why the Content-Disposition information is placed in a response _header_.

I’ve reviewed the relevant specs and can find no suggestion that a response containing a Content-Disposition header field should be treated as navigation. To the contrary, RFC6266 specifically states that the recipient should prompt the user to save the response locally, _rather than process it normally_.

https://tools.ietf.org/html/rfc6266#section-4.2
Use of the Content-Disposition Header Field in the Hypertext Transfer Protocol (HTTP):

4.2.  Disposition Type
If the disposition type matches "attachment" (case-insensitively), this indicates that the recipient should prompt the user to save the response locally, rather than process it normally (as per its media type).

https://tools.ietf.org/html/rfc2616#section-15.5
RFC2616 Hypertext Transfer Protocol -- HTTP/1.1 says: 

19.5.1 Content-Disposition:

The Content-Disposition response-header field has been proposed as a means for the origin server to suggest a default filename if the user requests that the content is saved to a file. This usage is derived from the definition of Content-Disposition in RFC 1806 [35].

content-disposition = "Content-Disposition" ":" disposition-type *( ";" disposition-parm )
        disposition-type = "attachment" | disp-extension-token
        disposition-parm = filename-parm | disp-extension-parm
        filename-parm = "filename" "=" quoted-string
        disp-extension-token = token
        disp-extension-parm = token "=" ( token | quoted-string )

An example is: Content-Disposition: attachment; filename="fname.ext"

The receiving user agent SHOULD NOT respect any directory path information present in the filename-parm parameter, which is the only parameter believed to apply to HTTP implementations at this time. The filename SHOULD be treated as a terminal component only.

If this header is used in a response with the application/octet-stream content-type, the implied suggestion is that the user agent should not display the response, but directly enter a 'save response as...' dialog.
> I would ask you to please describe how a client JavaScript implementation
> that makes AJAX calls to web services properly should request that a file 
> be downloaded from the server so that AJAX transactions are not affected?

The simplest cross-browser way for modern browsers is to use an <a download="somefilename"> and click() it.  Since the browser knows up front that will result in a download, it knows it can't possibly be a navigation.

This obviously doesn't work if the file data to be downloaded is computed based on a POST request...  It's possible to work around this by basically doing an XMLHttpRequest to get the data yourself, then stuffing it into a Blob and using URL.createObjectURL to create a URL that you can then use with <a download>.  The drawback there is that if the file to be downloaded is large you end up using a lot of RAM in the process.

Actually, the really really simplest way if a form has to be involved, if you know for a fact that the server will respond with a file download, is to target the corresponding form submission to a subframe like so:

  <iframe style="display: none" name="downloader"></iframe>
  <form action="whatever" target="downloader">stuff</form>

This will end up canceling all the network activity in the iframe, but there's nothing there to start with, so that's ok.

> _before_ canceling their AJAX calls

Yes, but the big question is when exactly they cancel them after that point.  We know some things (like doing it after you fire the unload event) are not web-compatible, for example.

> I’ve reviewed the relevant specs and can find no suggestion that a response
> containing a Content-Disposition header field should be treated as navigation.

OK, let's go through this step by step.

1. You're submitting a form.  That starts you off at https://html.spec.whatwg.org/multipage/forms.html#form-submission-algorithm

2. Step 18 of that algorithm queues a task to invoke "Navigate", which jumps you to <https://html.spec.whatwg.org/multipage/browsers.html#navigate>.  This is what "navigation" means in this discussion: invocation of this algorithm.

3. Step 8 of the Navigate algorithm does "Abort the active document", which calls into https://html.spec.whatwg.org/multipage/browsers.html#abort-a-document

4. Step 2 of "Abort the active document" cancels all network activity.

Note that the actual HTTP request is not sent until step 12 of the Navigate algorithm, and the HTTP response is not received until after that, which is all later than the "Abort the active document" in step 8 of Navigate.
(Reporter)

Comment 19

a year ago
Boris-

Thanks for your suggested work-arounds. 

Relevance Suite (R) loads and remains on a single page, whose URL never changes.  It uses AJAX requests to modify the contents of that single page in response to user events. Because the "file" being downloaded is actually content entirely generated by an MVC controller from a database and written directly to the response buffer without ever being written to disk, we can't use the <a download="somefilename"> approach.  

I considered using the Blob and URL.createObjectURL approach, but viewed that as a last resort if we couldn't get the form approach to work across all current mainstream browsers.

"Target the form to a hidden iframe" is a clever kludge that required only 2 additional lines of JavaScript to implement within our download() function. It successfully works around the problem in Firefox, without causing problems in Chrome, MS Edge or Safari. Even though we already use a blackhole <div> to route discardable AJAX responses or to measure a given font string's length in pixels, I didn't understand this issue in Firefox well enough to consider using it for this purpose, so I thank you for the suggestion. 

I also reviewed the spec documents you identified, and you're correct about the current algorithm. However, I do think the spec should be changed to adopt the algorithms implemented by Google Chrome and MS Edge, since they both manage to deliver the expected behavior (not canceling the current page when the form POST results in a response that contains Content-Disposition: attachment).  I suspect the current spec was written long before the modern design of using AJAX to update the current page became popular.

How should this discussion morph into a spec issue?
Eric, I'm glad the hidden iframe thing worked well for you.

> How should this discussion morph into a spec issue?

Probably by filing an issue at https://github.com/whatwg/html/issues if there isn't one already.  At first glance, https://github.com/whatwg/html/issues/1382 might be more or less about the same thing, maybe.
(Reporter)

Comment 21

a year ago
Boris, i think issue 1382 seems spot on, and I've added my 2 cents comment to it.  I don't believe the iFrame workaround should be necessary to avoid this issue, but it clearly works for us.

Thanks very much for your help!
(Assignee)

Comment 22

a year ago
I think we can close this bug now. Thanks for bz's detailed explanation.
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Flags: needinfo?(efriedm)
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.