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

assert_throws_js fails for an element adopted from an iframe #45405

Open
mbrodesser-Igalia opened this issue Mar 28, 2024 · 9 comments
Open

assert_throws_js fails for an element adopted from an iframe #45405

mbrodesser-Igalia opened this issue Mar 28, 2024 · 9 comments
Labels
resources testharness.js

Comments

@mbrodesser-Igalia
Copy link
Contributor

mbrodesser-Igalia commented Mar 28, 2024 •
edited
Loading

Example:

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8" />
    <meta
      http-equiv="Content-Security-Policy"
      content="require-trusted-types-for 'script';"
    />
    <script src="/resources/testharness.js"></script>
    <script src="/resources/testharnessreport.js"></script>
  </head>
  <body>
    <iframe id="otherIframe"></iframe>
    <script>
      const passThroughPolicy = trustedTypes.createPolicy("passThrough", {
        createHTML: (s) => s,
      });

      async_test((t) => {
        const sourceFrame = document.createElement("iframe");

        sourceFrame.srcdoc = passThroughPolicy.createHTML(
          `<!DOCTYPE html>
            <head>
            <meta charset="utf-8">
            </head>
            <body><iframe srcdoc="v">
            doc without TT CSP.
            </body>`
        );

        sourceFrame.addEventListener(
          "load",
          t.step_func_done(() => {
            const sourceElement =
              sourceFrame.contentDocument.body.querySelector("iframe");
            const sourceAttr = sourceElement.getAttributeNode("srcdoc");
            sourceElement.removeAttributeNode(sourceAttr);

            document.body.append(sourceElement);

            assert_throws_js(TypeError, () => {
              const otherIframe = document.getElementById("otherIframe");
              // This should throw because trusted types are enabled.
              otherIframe.setAttributeNode(sourceAttr);
            }); // works

            assert_throws_js(TypeError, () => {
              // This should throw because trusted types are enabled.
              sourceElement.setAttributeNode(sourceAttr);
            }); // fails
          })
        );

        document.body.append(sourceFrame);
      }, `x`);
    </script>
  </body>
</html>

Error log:

Unexpected subtest result in /trusted-types/x.html:
  │ FAIL [expected PASS] x
  │   → assert_throws_js: function "() => {
              // This should throw because trusted types are enabled.
              sourceElement.setAttributeNode(sourceAttr);
            }" threw object "TypeError: Failed to execute 'setAttributeNode' on 'Element': This document requires 'TrustedHTML' assignment." ("TypeError") expected instance of function "function TypeError() { [native code] }" ("TypeError")
  │ 
  │     at Test.<anonymous> (http://web-platform.test:8000/trusted-types/x.html:47:13)
  │     at Test.step (http://web-platform.test:8000/resources/testharness.js:2622:25)
  └     at HTMLIFrameElement.<anonymous> (http://web-platform.test:8000/resources/testharness.js:2697:32)

  [1/1] No tests running.

Reproducible with Chrome; Firefox doesn't support Trusted Types yet.

The failure stems from https://searchfox.org/mozilla-central/rev/b4860b54810539f0e4ab1fc46a3246daf2428439/testing/web-platform/tests/resources/testharness.js#2122.

CC @lukewarlow
mbrodesser-Igalia added a commit to mbrodesser-Igalia/wpt that referenced this issue Mar 28, 2024
@mbrodesser-Igalia
Change test to check setAttributeNode and setAttribute respect th…
9839909 
…e element's node document's global's CSP

Requires web-platform-tests#45405 to be
fixed.
@mbrodesser-Igalia mbrodesser-Igalia mentioned this issue Mar 28, 2024
Merged
@mbrodesser-Igalia
Copy link
Contributor Author

When debugging this, be aware that #44352 will prevent the above error log.

@mbrodesser-Igalia
Copy link
Contributor Author

Adding

      console.log(
        "(e.constructor === constructor)=" + (e.constructor === constructor)
      );
      console.log("(e instanceof TypeError)=" + (e instanceof TypeError));
      console.log("e.constructor.name=" + e.constructor.name);

to https://searchfox.org/mozilla-central/rev/b4860b54810539f0e4ab1fc46a3246daf2428439/testing/web-platform/tests/resources/testharness.js#2122 reveals:

 "(e.constructor === constructor)=true", source: http://web-platform.test:8000/resources/testharness.js (2408)
 "(e instanceof TypeError)=true", source: http://web-platform.test:8000/resources/testharness.js (2411)
 "e.constructor.name=TypeError", source: http://web-platform.test:8000/resources/testharness.js (2412)

 "(e.constructor === constructor)=false", source: http://web-platform.test:8000/resources/testharness.js (2408)
 "(e instanceof TypeError)=false", source: http://web-platform.test:8000/resources/testharness.js (2411)
 "e.constructor.name=TypeError", source: http://web-platform.test:8000/resources/testharness.js (2412)

@ptomato
Copy link
Contributor

ptomato commented Mar 28, 2024

It may be relevant for you, test262 takes this case into account in its assert.throws() function with a specific error message: https://github.com/tc39/test262/blob/837def66ac1700ce398df4a3b4eaeea915aa1df5/harness/assert.js#L87-L94

@Ms2ger
Copy link
Contributor

Ms2ger commented Mar 28, 2024

This is unsurprising, yeah. Use otherIframe.contentWindow.TypeError or something like that

@mbrodesser-Igalia
Copy link
Contributor Author

This is unsurprising, yeah. Use otherIframe.contentWindow.TypeError or something like that

@Ms2ger: please explain why it's unsurprising. It's not obvious.

Using

assert_throws_js(otherIframe.contentWindow.TypeError, () => {
              // This should throw because trusted types are enabled.
              sourceElement.setAttributeNode(sourceAttr);
            }); // fails

still fails.

@mbrodesser-Igalia
Copy link
Contributor Author 

assert_throws_js(sourceFrame.contentWindow.TypeError

works.

@mbrodesser-Igalia
Copy link
Contributor Author

Admittedly, I still don't understand why so would be happy to learn about it.

mbrodesser-Igalia added a commit to mbrodesser-Igalia/wpt that referenced this issue Apr 8, 2024
@mbrodesser-Igalia
Change test to check setAttributeNode and setAttribute respect th…
b617318 
…e element's node document's global's CSP

Requires web-platform-tests#45405 to be
fixed.
@Ms2ger
Copy link
Contributor

Ms2ger commented Apr 8, 2024

Every realm has its own set of intrinsics, including TypeError, and JavaScript does not go behind the scenes to match those up when you do instanceof. So if you have

error from page A → TypeError from page A
error from page B → TypeError from page B

you should by default not assume there's a connection between error from page A and TypeError from page B. I hope that clarifies a bit.

@mbrodesser-Igalia
Copy link
Contributor Author

Every realm has its own set of intrinsics, including TypeError, and JavaScript does not go behind the scenes to match those up when you do instanceof. So if you have

error from page A → TypeError from page A
error from page B → TypeError from page B

you should by default not assume there's a connection between error from page A and TypeError from page B. I hope that clarifies a bit.

@Ms2ger: a bit. None of the documentation I read implied that, or I was unable to deduce those implications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
resources testharness.js
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Ms2ger @gsnedders @ptomato @mbrodesser-Igalia