Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Incorrect Handling of "<" and ">" in Script Comments (DOMParser and Window) #1564

Open
cf-amin-aoulkadi opened this issue Oct 11, 2024 · 9 comments
Open

Incorrect Handling of "<" and ">" in Script Comments (DOMParser and Window) #1564

cf-amin-aoulkadi opened this issue Oct 11, 2024 · 9 comments
Labels
bug Something isn't working

Comments

@cf-amin-aoulkadi
Copy link

Describe the bug
If a <script> element contains a JavaScript comment (see example below), < and > characters within that comment are converted to &lt; and &gt; by Happy DOM. This should not happen.

Example:

<div>
  <script>
    //<>&lt;&gt;
  </script>
</div>

To Reproduce

  1. Run the following code using a DOMParser provided by Happy DOM: 
    console.log(new DOMParser().parseFromString("<div><script>//<>&lt;&gt;</script></div>", "text/html").body.outerHTML);
  2. The output (with Happy DOM 15.7.4) is: 
    <body><div><script>//&lt;&gt;&lt;&gt;</script></div></body>
    Note that //<>&lt;&gt; has been converted to //&lt;&gt;&lt;&gt;.

I have also seen this happen when setting window.document.body.innerHTML of a Happy DOM Window.

Expected behavior
Firefox and Chrome developer consoles generate the following output:

<body><div><script>//<>&lt;&gt;</script></div></body>

Device:

  • OS: Debian GNU/Linux 11 (bullseye) (mcr.microsoft.com/devcontainers/typescript-node:1-20-bullseye dev container)
  • Browser: Node.js
  • Version: 20.13.1

Additional context
I am using Happy DOM with Vitest.

This is my Happy DOM Window configuration:

const window = new Window({
  settings: {
    disableJavaScriptEvaluation: true
  }
});
@cf-amin-aoulkadi cf-amin-aoulkadi added the bug Something isn't working label Oct 11, 2024
@OlaviSau
Copy link
Contributor

OlaviSau commented Oct 16, 2024 •
edited
Loading

This is quite a major problem - it breaks css / scripts. @capricorn86

I think the issue comes from XMLSerializer escaping all text even in CSS / Script context, there should be a css / script aware escape or no escape at all.

            case NodeTypeEnum.textNode:
                return Entities.escapeText(root.textContent);

@Slimacik
Copy link

Slimacik commented Nov 8, 2024 •
edited
Loading

Also rendering of links is affected:
This code will
new DOMParser().parseFromString("<a href='/test?at1=val1&at2=val2'>Link</a>", "text/html").body.innerHTML
will render incorrect link:
<a href="/test?at1=val1&amp;at2=val2">Link</a>

@cf-amin-aoulkadi
Copy link
Author

@Slimacik I just tested your code in the developer consoles of Firefox and Chrome. Both consoles produced the same output as Happy DOM, so the behavior you are describing seems to be correct.

@capricorn86
Copy link
Owner

There was a fix related to this in v15.10.5. Does the problem still exists?

@Slimacik
Copy link

Slimacik commented Nov 15, 2024 •
edited
Loading

@capricorn86 I just tested with v15.10.5 as well as v15.11.6. My tests failed with both versions as link's href contains &amp; instead of & in query part of url

It works with v14.12.3 though

@Slimacik
Copy link

@cf-amin-aoulkadi Can you describe how did you tested it please? I just executed lines below in Chrome's dev. console and got href with & and not with &amp;

Executed lines:

var elm = document.createElement("a");
elm.href = "/test?attr1=val1&attr2=va2";
console.log(elm.href);

@cf-amin-aoulkadi
Copy link
Author

@Slimacik I simply ran this code from your first comment in the developer console ...

new DOMParser().parseFromString("<a href='/test?at1=val1&at2=val2'>Link</a>", "text/html").body.innerHTML

... and the browser returned '<a href="/test?at1=val1&amp;at2=val2">Link</a>' .

@cf-amin-aoulkadi
Copy link
Author

There was a fix related to this in v15.10.5. Does the problem still exists?

The problem I initially reported (<> being converted to &lt;&gt;) is technically fixed as of 15.10.5.

However, 15.10.5 basically flipped the problem around: Now &lt;&gt; is converted to <> (which is also wrong).

Reproduction

new DOMParser().parseFromString("<div><script>//<>&lt;&gt;</script></div>", "text/html").body.outerHTML

Expected Result (Firefox / Chrome): <body><div><script>//<>&lt;&gt;</script></div></body>
Happy DOM: <body><div><script>//<><></script></div></body>

@Slimacik
Copy link

@cf-amin-aoulkadi You are right, thank you! I did not realised that link in innerHTML can be different than value retrieved by href attribute, and it worked with previous versions of happy dom.
I did rewrite my tests to test href values instead of innerHTML, and it works now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Slimacik @OlaviSau @capricorn86 @cf-amin-aoulkadi