Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

jsdom crash: Cannot call method 'appendChild' of null #389

Closed
lbdremy opened this Issue · 22 comments
@lbdremy

documentElement is null because the HTML retrieved does not contain the body tag, so the first time that the appendChild() function is called to append the script tag, the error occurs. I know that's madness but it happens. :S
How do you want to handle this case?

I'm going to attach a test for this issue.

@lbdremy lbdremy referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@tmpvar
Owner

in your test, <html> should be the documentElement no?

I think what you may have meant is that since there is no body, you cant add a script to it. I think in this case we should ensure a body exists before inserting the script tag.

@lbdremy lbdremy referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@lbdremy

The html tag should be the documentElement but here window.document.documentElement is null, the html tag is present and the only tag missing in this piece of html is the body tag.

If the body tag is present in the html page window.document.documentElement is not null.
So it appears that something goes wrong for the window.document.documentElement if the body tag is missing and this should not occur.
Do you agree or I missed something?

the error log of the test:

✖ jsdom/index/issue_389_documentElement_is_null
Cannot call method 'appendChild' of null TypeError: Cannot call method 'appendChild' of null
    at /home/lbdremy/workspace/nodejs/jsdom/lib/jsdom.js:249:41
    at Array.forEach (native)
    at /home/lbdremy/workspace/nodejs/jsdom/lib/jsdom.js:231:18
    at Object.env (/home/lbdremy/workspace/nodejs/jsdom/lib/jsdom.js:261:5)
    at Object.<anonymous> (/home/lbdremy/workspace/nodejs/jsdom/test/jsdom/index.js:1263:11)
    at Object.<anonymous> (/home/lbdremy/workspace/nodejs/jsdom/node_modules/nodeunit/lib/core.js:233:16)
    at /home/lbdremy/workspace/nodejs/jsdom/node_modules/nodeunit/lib/core.js:233:16
    at Object.runTest (/home/lbdremy/workspace/nodejs/jsdom/node_modules/nodeunit/lib/core.js:69:9)
    at /home/lbdremy/workspace/nodejs/jsdom/node_modules/nodeunit/lib/core.js:115:25
    at /home/lbdremy/workspace/nodejs/jsdom/node_modules/nodeunit/deps/async.js:508:13
    at /home/lbdremy/workspace/nodejs/jsdom/node_modules/nodeunit/deps/async.js:118:13
    at /home/lbdremy/workspace/nodejs/jsdom/node_modules/nodeunit/deps/async.js:129:25
    at /home/lbdremy/workspace/nodejs/jsdom/node_modules/nodeunit/deps/async.js:510:17
    at Array.<anonymous> (/home/lbdremy/workspace/nodejs/jsdom/node_modules/nodeunit/lib/types.js:146:17)
    at EventEmitter._tickCallback (node.js:126:26) Error
    at /home/lbdremy/workspace/nodejs/jsdom/test/runner:185:58
    at Array.forEach (native)
    at Object.testDone (/home/lbdremy/workspace/nodejs/jsdom/test/runner:170:18)
    at Array.0 (/home/lbdremy/workspace/nodejs/jsdom/node_modules/nodeunit/lib/types.js:145:25)
    at EventEmitter._tickCallback (node.js:126:26)

@tmpvar
Owner

I completely agree, <html> is the documentElement in html documents.

@lbdremy lbdremy referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@romansky

Any updates with this issue? still happening to me..

@philpoore

Happening to me too... :(

@philpoore

├─┬ jsdom@0.2.15
│ ├─┬ contextify@0.1.3
│ │ └── bindings@1.0.0
│ ├── cssom@0.2.5
│ └── htmlparser@1.7.6

@christopherchiu

Is there a fix/workaround for this issue?

@domenic
Collaborator

Sounds like something worth prioritizing for the next release. Possibly parsing-related—shouldn't the parser give us an empty body tag in this case?

@eamodeorubio

Hi, same problem here, but I think I've got a hint. If you use jsdom to parse http://richskinnypretty.com/ you get this error, but if you parse the following http://richskinnypretty.blogspot.com.es/ you don't
The former has iframes, the later doesn't. So I think the problem is parsing a document with iframes

@domenic
Collaborator

This issue no longer contains a test case, so I can't really see if it's fixed or how to fix it. But, very soon we're going to have a new release with a new HTML parser that will likely not have this problem.

@domenic domenic closed this
@pdelanauze

+1 , just experienced this issue with release 0.6.1

@domenic
Collaborator

@pdelanauze were you able to create a test case we can reproduce this with?

@fb55

@domenic #617 also suffers from this problem. Haven't tracked it down yet.

@tamilvendhank

+1, I am also experiencing this.

@domenic
Collaborator

@tamilvendhank as with everyone else in this thread, I will ask: were you able to create a test case we can reproduce this with?

@tamilvendhank

I am running behind deadline now. But, I will definitely create a test case in the weekend. Is there any template available for creating testcase?

@domenic
Collaborator

Something like this would work:

https://github.com/tmpvar/jsdom/blob/master/test/jsdom/parsing.js#L136-L146

Basically just find a string that you can do var doc = jsdom(theString) and then do some minimal operations that trigger the error.

@tamilvendhank

ok, I will do it. Thanks!

@jhnns

I've created a minimal test-case that triggers the error:

jsdom.env({
    html: "<html></html>",
    src: "''",
    done: function () { console.log("done"); }
});
/media/sf_dev/test/node_modules/jsdom/lib/jsdom.js:294
      window.document.documentElement.appendChild(script);
                                      ^
TypeError: Cannot call method 'appendChild' of null
    at /media/sf_dev/test/node_modules/jsdom/lib/jsdom.js:294:39

The html has no body-element which causes jsdom to break when the Javascript-src '' is appended to document.documentElement

@domenic
Collaborator

Ugh, another one caused by the parser not handling empty documents correctly :(

@domenic domenic reopened this
@jhnns

:+1:

Many ad networks are using these empty documents to do all kind of iframe magic.

@domenic domenic referenced this issue
Closed

Add tests for all outstanding [parsing] issues #829

14 of 14 tasks complete
@domenic domenic closed this in b7d69c5
@jhnns

Cool! Thx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.