This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 9767 - Consider ignoring document.write() when IE ignores it if comes from the network task source
Summary: Consider ignoring document.write() when IE ignores it if comes from the netwo...
Status: RESOLVED FIXED
Alias: None
Product: HTML WG
Classification: Unclassified
Component: pre-LC1 HTML5 spec (editor: Ian Hickson) (show other bugs)
Version: unspecified
Hardware: All All
: P1 critical
Target Milestone: ---
Assignee: Ian 'Hixie' Hickson
QA Contact: HTML WG Bugzilla archive list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-19 07:43 UTC by Henri Sivonen
Modified: 2010-10-12 21:20 UTC (History)
9 users (show)

See Also:


Attachments

Description Henri Sivonen 2010-05-19 07:43:53 UTC
At bugzilla.mozilla.org, we've gotten a very small but non-zero number of reports about build with the HTML5 parser enabled showing blank pages.

In all cases, this has been about document.write() without an insertion point blowing away the document. The dominant pattern is that there's browser sniffing and the code path for IE does document.write("<script src='another-script-that-calls-document-write.js'></script>"); and the code path for Gecko does var s = document.createElement("script"); s.src = "another-script-that-calls-document-write.js"; document.body.appendChild(s);.

Thus, the problem would go away if the sites removed sniffing and used the IE-targeted loading method for all browsers. (There's also been one report of MITM malware injecting this problem to pages.)

However, experimentation shows that IE itself mitigates the problem compared to what the spec says by ignoring document.write() in some cases.
See https://bug560256.bugzilla.mozilla.org/attachment.cgi?id=446166 for an example.

Ignoring document.write() without a defined insertion point during the parse would not be a nice solution, because it would introduce a race between asynchronous script loads and the parse finishing. Thus, authors whose network made the script always load before the parser is done might not notice a problem but users whose network makes the script fire after the parser is done would get the document blown away.

I would guess there's a compat reason why document.write() after the parser is done implies document.open(), so I suppose always ignoring document.write() without a defined insertion point isn't a solution, either.

At least at this point, I haven't worked out what criteria IE uses.

I'm kinda expecting that the outcome here is that there is no non-racy solution that isn't overly complex and this will be WONTFIX. However, I'm filing this bug in case I'm missing something. If there is a sanely implementable non-racy condition when IE ignores document.write(), it might be worthwhile to spec it.
Comment 1 Henri Sivonen 2010-05-19 15:51:50 UTC
Potential solution: Ignoring document.write() if script is running from the network task source due to external script element completing fetch (as opposed to event handlers or timeouts) and the insertion point is undefined.
Comment 2 Jonas Sicking (Not reading bugmail) 2010-05-19 17:07:40 UTC
That sounds like a possible solution. Though I think I'd like to wait a bit longer before making changes to the spec until we see that it's really necessary. So far we've just heard of two sites breaking.
Comment 3 Henri Sivonen 2010-05-20 13:18:24 UTC
(In reply to comment #2)
> That sounds like a possible solution. Though I think I'd like to wait a bit
> longer before making changes to the spec until we see that it's really
> necessary. So far we've just heard of two sites breaking.

Ignoring document.write() coming from a <script src> completing fetch when the insertion point is undefined would mitigate 4 out of the total 6 open HTML5 parser evang bugs:
https://bugzilla.mozilla.org/show_bug.cgi?id=542104
https://bugzilla.mozilla.org/show_bug.cgi?id=553795
https://bugzilla.mozilla.org/show_bug.cgi?id=565938
https://bugzilla.mozilla.org/show_bug.cgi?id=527635
(Additionally, there was the case of content-injecting [ad injecting?] router-based MITM malware.)

That is, the result wouldn't be what the site meant but the effect on the user would be missing out on some ads instead of getting a blank page.

(The 2 other HTML5 parser evang bugs are
https://bugzilla.mozilla.org/show_bug.cgi?id=540480
https://bugzilla.mozilla.org/show_bug.cgi?id=565689 )
Comment 4 Jonas Sicking (Not reading bugmail) 2010-05-20 19:03:01 UTC
I could live with this change I guess, but so far I could also live with not making any change.
Comment 5 Tony Gentilcore 2010-06-16 23:31:56 UTC
After enabling the HTML5 parser in WebKit, the Chromium reliability tests found a bunch more sites where this is a problem. This WebKit bug lists several of them: https://bugs.webkit.org/show_bug.cgi?id=40745
Comment 6 Jonas Sicking (Not reading bugmail) 2010-06-17 08:21:45 UTC
Has it been verified that all those sites would be fixed by implementing the suggestion in comment 1?
Comment 7 Adam Barth 2010-06-18 04:03:07 UTC
AFAIK, these sites are all trying to call document.write after we've received EOF from the network but before the parser has consumed the EOF character.  The previous WebKit behavior (which has been shipping for a while) was to insert the written characters just before the EOF character.  In some sense, the insertion point used to stay just before EOF until the EOF was actually consumed by the parser.
Comment 8 Henri Sivonen 2010-06-21 08:23:43 UTC
(In reply to comment #1)
> Potential solution: Ignoring document.write() if script is running from the
> network task source due to external script element completing fetch (as opposed
> to event handlers or timeouts) and the insertion point is undefined.

This condition is too broad because it would cause document.write() to be ignored when an external script call document.write on another document (e.g. an iframe child).

The Gecko patch that I wrote for this (neither reviewed nor landed) increments a counter on the owner document of an external script right before evaluating the external script and decrements the counter on the same document after (the owner might have changed by now).

document.write() on occasions where previously a call to document.open() was implied checks the above-mentioned counter on the document object first first. If the counter is zero, a call to document.open() is implied as before. Otherwise, document.write() returns right away and a warning is emitted to the JS console.

(It seems to me that the counter should never be able to get values other than 0 or 1, but I can imagine alert() or sync XHR causing situations I haven't thought through properly.)
Comment 9 Adam Barth 2010-07-09 05:24:18 UTC
Another example: http://www.voila.fr/
Comment 10 Ian 'Hixie' Hickson 2010-07-09 18:49:46 UTC
That's a tough one. I think ignoring document.write() in scripts that are asynchronously fetched for <script> elements inserted while the parser is running, rather than them blowing the document away is reasonable. I'm not sure that exactly maps to the explanation above, though. It would need a flag on the <script> element set when the element is added to the document, based on whether the parser is on the stack, and then for the script execution to set a similar flag (using a counter as described above to be reentrancy-proof) that causes document.write() to bail.
Comment 11 Adam Barth 2010-07-09 19:14:29 UTC
Is there some reason you don't want to insert the document.write before the EOF character?  Since we haven't consumed the EOF character yet, this should be nice and predictable.  We could do that whenever we get a document.write without an insertion point.  There's some reason to believe this works for the web because it's what WebKit used to do in these situations.
Comment 12 Henri Sivonen 2010-07-10 06:37:04 UTC
(In reply to comment #11)
> Is there some reason you don't want to insert the document.write before the EOF
> character?

It would complicate the off-the-main-thread speculation machinery in Gecko. Now EOF handling can run on the parser thread without any fancy coordination with the main thread.
Comment 13 Henri Sivonen 2010-07-10 06:50:27 UTC
(In reply to comment #10)
> That's a tough one. I think ignoring document.write() in scripts that are
> asynchronously fetched for <script> elements inserted while the parser is
> running, rather than them blowing the document away is reasonable. I'm not sure
> that exactly maps to the explanation above, though. It would need a flag on the
> <script> element set when the element is added to the document, based on
> whether the parser is on the stack, and then for the script execution to set a
> similar flag (using a counter as described above to be reentrancy-proof) that
> causes document.write() to bail.

That seems overly complex to the solution in comment 8. Is there a reason why ignoring document.write()s from scripts inserted after the end of parse is unreasonable (when the owner doc of the script is the document write() is called on)?
Comment 14 Adam Barth 2010-07-10 07:08:54 UTC
> It would complicate the off-the-main-thread speculation machinery in Gecko. Now
> EOF handling can run on the parser thread without any fancy coordination with
> the main thread.

I don't really understand the constraints imposed by that architecture.  I'm not sure it's a good idea to ignore these writes.  They could contain markup that's important to the functioning of the page.

Wouldn't you just need to round-trip to the main thread before processing EOF?  That doen't seem like very fancy coordination.
Comment 15 Adam Barth 2010-07-11 20:50:23 UTC
http://www.nakedcapitalism.com/

This bug affects too many sites.  We cannot ship the specced behavior.  I feel bad shipping it nightlies.
Comment 16 Adam Barth 2010-07-13 00:53:28 UTC
I implemented the following behavior in WebKit:

1) If we receive a document.write without a current insertion point, we attempt to append it to the end of the input steam.
2) If (1) would result in appending to the input stream after the EOF character, then we ignore the call to document.write.

This is similar what Henri suggests.  The mental model is that we always "append" if there's no current insertion point, but appending after the EOF character is pretty useless since the EOF character stops the parsing algorithm.  :)

Hopefully we'll get some data about whether this approach is sufficiently compatible.
Comment 17 Ian 'Hixie' Hickson 2010-07-14 02:02:32 UTC
The solution in comment 8 seems to make document.write()s in timeouts act differently based on whether the parser has finished or not, which seems bad.

The solution in comment 16 makes the resulting document depend on the network. It's bad enough that we let script load order depend on the network, but having it change the resulting source document seems excessively bad. It also seems like this would still cause the page to go blank in the case of the scripts taking longer to load than the page (i.e. nothing seems to prevent the scripts from being read after the EOF is consumed). In fact I don't really follow how you're preventing the EOF from being consumed at all... surely as soon as you know there's an EOF, it's consumed? Are you just appending it at random points in the stream (i.e. after whatever you happen to have received from the network so far)?

EDITOR'S RESPONSE: This is an Editor's Response to your comment. If you are satisfied with this response, please change the state of this bug to CLOSED. If you have additional information and would like the editor to reconsider, please reopen this bug. If you would like to escalate the issue to the full HTML Working Group, please add the TrackerRequest keyword to this bug, and suggest title and text for the tracker issue; or you may create a tracker issue yourself, if you are able to do so. For more details, see this document:
   http://dev.w3.org/html5/decision-policy/decision-policy.html

Status: Accepted
Change Description: see diff given below
Rationale: The problem is real; I've gone with the solution in comment 10 for now.
Comment 18 contributor 2010-07-14 02:02:54 UTC
Checked in as WHATWG revision r5157.
Check-in comment: An attempt at making non-parser scripts inserted while the parser is running fail at document.write() rather than blowing away the document.
http://html5.org/tools/web-apps-tracker?from=5156&to=5157
Comment 19 Adam Barth 2010-07-14 17:40:57 UTC
> Are you just appending it at random points
in the stream (i.e. after whatever you happen to have received from the network
so far)?

Yes.  That's how the old WebKit parser used to work.

I'll take a crack at implementing your solution in WebKit and will let you know if there are any problems.  Thanks.
Comment 20 Henri Sivonen 2010-07-15 11:57:54 UTC
(In reply to comment #17)
> The solution in comment 8 seems to make document.write()s in timeouts act
> differently based on whether the parser has finished or not, which seems bad.

How so? What I suggested in comment 8 (and just landed, btw) doesn't change document.write() behavior from timeouts. With comment 8, a document.write() from a timeout still blows away the document, always.

> The solution in comment 16 makes the resulting document depend on the network.
> It's bad enough that we let script load order depend on the network, but having
> it change the resulting source document seems excessively bad. It also seems
> like this would still cause the page to go blank in the case of the scripts
> taking longer to load than the page (i.e. nothing seems to prevent the scripts
> from being read after the EOF is consumed). In fact I don't really follow how
> you're preventing the EOF from being consumed at all... surely as soon as you
> know there's an EOF, it's consumed? Are you just appending it at random points
> in the stream (i.e. after whatever you happen to have received from the network
> so far)?

FWIW, I think we shouldn't stuff right before EOF. There's really no point in having that kind of complexity when the offending scripts are ads or analytics anyway, so ignoring the writes doesn't degrade the user-perceived browser behavior at all. Ignoring a write from an analytics script doesn't make end users reject a browser upgrade, so changing behavior towards sanity should be safe in terms of market share.

> EDITOR'S RESPONSE: This is an Editor's Response to your comment. If you are
> satisfied with this response, please change the state of this bug to CLOSED. If
> you have additional information and would like the editor to reconsider, please
> reopen this bug. If you would like to escalate the issue to the full HTML
> Working Group, please add the TrackerRequest keyword to this bug, and suggest
> title and text for the tracker issue; or you may create a tracker issue
> yourself, if you are able to do so. For more details, see this document:
>    http://dev.w3.org/html5/decision-policy/decision-policy.html
> 
> Status: Accepted
> Change Description: see diff given below
> Rationale: The problem is real; I've gone with the solution in comment 10 for
> now.

I'm unhappy with the specced solution. Write-neutralization depends on there being an active parser, which makes write-neutralization racy with the end of the parse. I still think we should go with comment 8, which I believe Hixie misunderstood, given the comment above talking about timeouts when comment 8 had no effect on timeouts.

Comment 8 basically write-neutralizes all external scripts that
 1) Aren't parser-inserted
AND
 2) have the same owner document as the document that write is being called on

This still leaves the implied open() for cases where one document overwrites another and where an external script overwrites its own document later from an event handler or timeout (as opposed to triggering write() as a side effect of the evaluation of the script when the script completes loading).
Comment 21 Ian 'Hixie' Hickson 2010-07-15 23:10:18 UTC
I don't understand how the spec is racy with the parse. Can you elaborate?

Your proposal seems to affect far more scripts than necessary. I hadn't realised you meant to affect so many scripts. Is that really necessary? Why would we want a timeout inserted an external script that uses document.write() to be ignored? Surely the ideal here is to have as small an effect on the overall model as is possible while still resolving the compatibility problem.
Comment 22 Henri Sivonen 2010-07-16 09:00:00 UTC
(In reply to comment #21)
> I don't understand how the spec is racy with the parse. Can you elaborate?

I didn't read the spec changes carefully enough.

> Your proposal seems to affect far more scripts than necessary. I hadn't
> realised you meant to affect so many scripts. Is that really necessary? Why
> would we want a timeout inserted an external script that uses document.write()
> to be ignored? Surely the ideal here is to have as small an effect on the
> overall model as is possible while still resolving the compatibility problem.

No, that's not the ideal. There are some cases that need write-neutralization for compat. Presumably, there are other situations that must not be write-neutralized for compat. 

It's possible that there's a set of cases in between that don't matter either way for compat. You are going for no change there just in case. My view is that the ideal is to go for simplicity in the cases that don't matter.

The problem is, of course, that I'm not sure if I have guessed the boundaries of the "don't matter" cases right.

Do you have data showing that sites depend on document.writes from timeout-inserted scripts blowing away the document? OTOH, I have seen a data point showing that it's desirable to write-neutralize external scripts that have the defer attribute set when they try to write in their owner doc.

While I'm well aware that Web sites depend on all sorts of crazy stuff, it still seems implausible that sites would depend on being able to blow self away by inserting a createElement-created external script. (OTOH, it seems totally plausible that sites are depending on the implied .open() behavior when calling .write() cross-browsing contexts.)
Comment 23 Jonas Sicking (Not reading bugmail) 2010-07-16 14:45:16 UTC
For what it's worth, I actually think it's a good idea to err on the side of making document.write() do nothing rather than blow away the entire page. This behavior seems like a much better error case for authors as a blank page is equally helpful as the yellow screen of death of XML fame.
Comment 24 Ian 'Hixie' Hickson 2010-08-16 21:12:42 UTC
So the proposal here is to change the spec so that all scripts running for <script> elements (when the element is run, not calls to functions created by that element and put in the scripting environment) that are using external files (i.e. <script src="">, not inline scripts) and that do not have the "parser-inserted" flag set (e.g. inserted using appendChild(), not found in the doc source), when they try to document.write() to the same Document as the owner of the <script> without having a defined insertion point, should have such calls ignored?

If so, some questions:

Why limit it to appendChild()'ed scripts, not parser-inserted ones? (e.g. async scripts don't have a defined insertion point)

Why limit it to external scripts?

Why limit it to when running <script> blocks? We could make all document.write() calls on your own document get ignored.
Comment 25 Henri Sivonen 2010-08-25 14:04:50 UTC
(In reply to comment #24)
> So the proposal here is to change the spec so that all scripts running for
> <script> elements (when the element is run, not calls to functions created by
> that element and put in the scripting environment) that are using external
> files (i.e. <script src="">, not inline scripts) and that do not have the
> "parser-inserted" flag set (e.g. inserted using appendChild(), not found in the
> doc source), when they try to document.write() to the same Document as the
> owner of the <script> without having a defined insertion point, should have
> such calls ignored?

Not exactly. What I implemented was that cases that would previously blow away the document instead ignore the write if an external script whose owner doc is the same doc as the one write is being called on is running.

> If so, some questions:
> 
> Why limit it to appendChild()'ed scripts, not parser-inserted ones? (e.g. async
> scripts don't have a defined insertion point)

I'm not suggesting limiting this to appendChild() scripts. I'm suggesting also ignoring writes on the owner doc from async and defer scripts.

> Why limit it to external scripts?

It was easy enough to limit it to external scripts and doing it for internal scripts didn't seem essential in order to avoid user-perceived breakage given the existing content I've seen, so I erred on the side if changing a smaller amount of behavior where scoping the behavior was reasonably simple.

> Why limit it to when running <script> blocks? We could make all
> document.write() calls on your own document get ignored.

Two reasons:
 1) It seemed unnecessary to have to be able to figure out from within the document.write() implementation if a calling event handler comes from the same doc. (I admit I wouldn't know right away how to do that in Gecko.)
 2) It seemed semi-plausible to me that some site out there might actually depend on being able to blow away self from an onclick handler.
Comment 26 Ian 'Hixie' Hickson 2010-09-25 14:51:41 UTC
So basically the proposal is to change this step:

 8. If the element's Document has an active parser, and the parser's script nesting level is non-zero, but this script element does not have the "parser-inserted" flag set, the user agent must set the element's "ignore-destructive-writes" flag.

...to:

 8. If the element has a "src" attribute, the user agent must set the element's "ignore-destructive-writes" flag.

...? That seems reasonable. (I'll probably simplify how it's specified, if we do this, but the effect would be the same.) Can you confirm that that is what you mean?
Comment 27 Henri Sivonen 2010-09-28 09:54:11 UTC
(In reply to comment #26)
> So basically the proposal is to change this step:
> 
>  8. If the element's Document has an active parser, and the parser's script
> nesting level is non-zero, but this script element does not have the
> "parser-inserted" flag set, the user agent must set the element's
> "ignore-destructive-writes" flag.
> 
> ...to:
> 
>  8. If the element has a "src" attribute, the user agent must set the element's
> "ignore-destructive-writes" flag.
> 
> ...? That seems reasonable. (I'll probably simplify how it's specified, if we
> do this, but the effect would be the same.) Can you confirm that that is what
> you mean?

That's not what I mean. I'm not suggesting elements to have an "ignore destructive writes" flag. I'm suggesting that documents have an "ignore destructive writes" counter. Furthermore, I'm not suggesting doing stuff at the time of the "run" algorithm but at the time of script evaluation.

Under http://www.whatwg.org/specs/web-apps/current-work/#executing-a-script-block

Change step 2 to:
If the script is from an external file, increment the "ignore-destructive-writes" counter on the Document of the script element. Let _neutralized doc_ be that Document.

Change step 4 to:
Decrement the "ignore-destructive-writes" counter of _neutralized doc_ if _neutralized doc_ was defined in the earlier step.

Under http://www.whatwg.org/specs/web-apps/current-work/#dom-document-write

Change step 2 to:
If the insertion point is undefined and the Document has the "ignore-destructive-writes" counter set to a non-zero value, then abort these steps.
And then say somewhere that a Document initially has its "ignore-destructive-writes" counter set to zero.

The solution described above has worked for multiple Firefox 4 betas. I really don't want to tweak it at this point without a very, very strong reason.
Comment 28 Ian 'Hixie' Hickson 2010-09-30 02:06:07 UTC
That seems to be identical to what I said, from a black box perspective. Am I missing something?
Comment 29 Henri Sivonen 2010-09-30 06:56:34 UTC
(In reply to comment #28)
> That seems to be identical to what I said, from a black box perspective. 

I'm not sure.

> Am I missing something?

What I said is conceptually simpler, since there are fewer interacting flags.

Also, in case what I said and what you said aren't equivalent, what I said has actually been tested in the wild.
Comment 30 Ian 'Hixie' Hickson 2010-09-30 07:30:21 UTC
Ok well I'm going to go on the basis that they're identical, spec what I said, and then simplify it, as per comment 26. Hopefully that will result in the spec basically saying what you proposed.
Comment 31 Henri Sivonen 2010-09-30 09:27:25 UTC
(In reply to comment #30)
> Ok well I'm going to go on the basis that they're identical, spec what I said,
> and then simplify it, as per comment 26. Hopefully that will result in the spec
> basically saying what you proposed.

Why don't you just spec what I proposed? It's conceptually simpler and has been tested.
Comment 32 Ian 'Hixie' Hickson 2010-10-12 21:20:19 UTC
Because just writing what you propose would mean I was specifying something I didn't understand. On the other hand, specifying something I _do_ understand and then simplifying it and reaching what you proposed (which is what I just did) means that I actually understand the spec and am confident that we were indeed proposing the same thing.

EDITOR'S RESPONSE: This is an Editor's Response to your comment. If you are satisfied with this response, please change the state of this bug to CLOSED. If you have additional information and would like the editor to reconsider, please reopen this bug. If you would like to escalate the issue to the full HTML Working Group, please add the TrackerRequest keyword to this bug, and suggest title and text for the tracker issue; or you may create a tracker issue yourself, if you are able to do so. For more details, see this document:
   http://dev.w3.org/html5/decision-policy/decision-policy.html

Status: Accepted
Change Description: see diff given below
Rationale: Compat.

Note: document.write() behaviour is so crazy at this point that I've added a warning discouraging authors from using it.
Comment 33 contributor 2010-10-12 21:20:52 UTC
Checked in as WHATWG revision r5616.
Check-in comment: Change how document.write() is ignored.
http://html5.org/tools/web-apps-tracker?from=5615&to=5616