Bug Tracker

Ticket #13976 (closed bug: fixed)

Opened 19 months ago

Last modified 15 months ago

Invalid HTML passed into .html() throws 'tmp is null' error

Reported by: patrick.guinness@… Owned by:
Priority: blocker Milestone: 2.0.3
Component: manipulation Version: 2.0.2
Keywords: Cc: orkel
Blocking: Blocked by:

Description

An error is thrown in jQuery 2.0.2, whereas jQuery 1.9.1 appears to strip out the offending tags.

2.0.2 example:  http://jsfiddle.net/pguinness/ndQ6R/2/

1.9.1 example:  http://jsfiddle.net/pguinness/axDzX/2/

Looks like the issue is occurring in the buildFragment() method. jQuery 1.9.1 runs the HTML content through the createSafeFragment() method (line 6564), whereas 2.0.2 uses the native .createDocumentFragment() (line 5779) without doing any additional parsing.

Tested on Mac OS 10.8.3 in the following browsers:

Chrome 27.0.1453.93 Firefox 21.0 Safari 6.0.4

Change History

comment:1 follow-up: ↓ 4 Changed 19 months ago by dmethvin

  • Status changed from new to open

I don't think we are necessarily going to guarantee the same output for garbage input across 1.x and 2.x, but we should avoid throwing an error.

comment:2 Changed 19 months ago by dmethvin

  • Priority changed from undecided to low
  • Component changed from unfiled to manipulation
  • Milestone changed from None to 2.next

comment:3 Changed 19 months ago by ruado1987

I just debugged the buildFragment function in both jQuery versions and came to believe that the error is not caused by different ways of creating documentFragment. In fact, the blame should be put on this line:

tmp = tmp.firstChild; 

In version 1.9.1, it is

tmp = tmp.lastChild; 

but somehow it was changed to the former. The latter works well because lastChild of tmp is a table and eventually tmp's value is tbody. On the other hand, firstChild of tmp is a text node which contains the garbage input and in the end tmp's value is null (because text node doesnt have any child). Since tmp is null, calling childNodes on it winds up in error.

comment:4 in reply to: ↑ 1 Changed 19 months ago by gibson042

Replying to dmethvin:

I don't think we are necessarily going to guarantee the same output for garbage input across 1.x and 2.x, but we should avoid throwing an error.

How committed are you to that stance? In this case, I actually consider the 2.x behavior to be an unintended benefit of its simpler wrapMap—the garbage input <#if><tr><p>This is a test.</p></tr><#/if> still becomes &lt;#if&gt;<p>This is a test.</p>&lt;#/if&gt;<table><tbody><tr></tr></tbody></table> in the fragment DOM (on my browser, at least), but helpfully yields an error instead of a meaningless and wrong equivalence with <tr></tr>.

comment:5 Changed 19 months ago by dmethvin

  • Cc markelog added
  • Milestone changed from 2.next to 2.0.3

My main concern is that it throws an error in one branch and not in the other. I gathered that the OP wanted to pass this in and didn't mind quietly getting garbage out because that's what we've done in the past. Throwing a hard-to-decypher error from within buildFragment is likely to be a bug report magnet.

@patrick, to be clear we're not "stripping out the tags" but the browsers are doing *something* with it and we aren't guaranteeing consistent output from bad input. There may still be situations where this would throw errors, for example XHTML documents.

I'm not sure if the change from .lastChild to firstChild was significant or not ... maybe orkel knows?

comment:6 Changed 18 months ago by timmywil

  • Priority changed from low to blocker

We seem to have agreed in the meeting that 2.x should not throw an error 1.x doesn't. Let's land a change for 2.0.3.

comment:7 Changed 18 months ago by ruado1987

  • Status changed from open to closed
  • Resolution set to fixed

Fixes #13976: Garbage input should not cause error in buildFragment. Close gh-1284

Changeset: 5f1aa2354aeffa787e40d44224a1b583b7c89666

comment:8 in reply to: ↑ description Changed 15 months ago by jamessungkhaphan@…

Replying to patrick.guinness@…:

An error is thrown in jQuery 2.0.2, whereas jQuery 1.9.1 appears to strip out the offending tags.

2.0.2 example:  http://jsfiddle.net/pguinness/ndQ6R/2/

1.9.1 example:  http://jsfiddle.net/pguinness/axDzX/2/

Looks like the issue is occurring in the buildFragment() method. jQuery 1.9.1 runs the HTML content through the createSafeFragment() method (line 6564), whereas 2.0.2 uses the native .createDocumentFragment() (line 5779) without doing any additional parsing.

Tested on Mac OS 10.8.3 in the following browsers:

Chrome 27.0.1453.93 Firefox 21.0 Safari 6.0.4

Note: See TracTickets for help on using tickets.