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

Error overlay html encodes the ansi html formatting the error output #3575

Closed
1 of 2 tasks
raix opened this issue Jul 29, 2021 · 6 comments · Fixed by #3579
Closed
1 of 2 tasks

Error overlay html encodes the ansi html formatting the error output #3575

raix opened this issue Jul 29, 2021 · 6 comments · Fixed by #3579

Comments

@raix
Copy link
Contributor

raix commented Jul 29, 2021 •
edited
Loading

  • This is a bug
  • This is a modification request

Code

Reproduced in the CRA webpack 5 update branch using webpack error overlay in the WP5 CRA pr (replacing the CRA error overlay)
(The "wp5" branch is on the way to "main")

Expected Behavior

image

Actual Behavior

image

For Bugs; How can we reproduce the behavior?

ansiHTML returns error formattet using html as a text string but because it's handed of to document.createTextNode the browser will escape the string problem is that errorMessage is already escaped (as it should be to prevent XSS/script injection) so no need for the double escaping.

Suggestion:
Change

...
      const text = ansiHTML(errorMessage);
      const messageTextNode = document.createTextNode(text);
...

To:

...
      var text = ansiHTML(encode(errorMessage));
      var messageTextNode = document.createElement('div');
      messageTextNode.innerHTML = text;
...

For Features; What is the motivation and/or use-case for the feature?
raix pushed a commit to facebook/create-react-app that referenced this issue Jul 29, 2021
Use webpack hot client and error overlay
5a9d460 
Currently a formatting issue in the overlay, ref: webpack/webpack-dev-server#3575
@raix raix mentioned this issue Jul 29, 2021
Merged
@erikt9
Copy link

erikt9 commented Jul 29, 2021

I believe this is a duplicate of #3552 which is already fixed (but not released).

@raix
Copy link
Contributor Author

raix commented Jul 29, 2021 •
edited
Loading

@erikt9 Im not sure that I agree... the work you refer to doesn't test or solve the escaping issue.

@raix
Copy link
Contributor Author

raix commented Jul 30, 2021 •
edited
Loading

From what I can tell of the #3553 it still use document.createTextNode(text) causing double escaping - so the PR #3553 removes the wrong escape method of the two imho
(it's browser behavior so the tests might not show the issue?)

cc. @snitin315 @alexander-akait

@snitin315
Copy link
Member

@raix Is it possible for you to provide a minimal reproduction repo for testing? create-react-app seems too large to test with.

@raix
Copy link
Contributor Author

raix commented Jul 31, 2021

@snitin315

For CRA:

  • git clone https://github.com/facebook/create-react-app.git
  • cd create-react-app
  • git checkout wp5
  • run yarn
  • run yarn start
  • edit the package/cra-template/template/src/App.js to trigger an error

Possible replication by adding this test to overlay.test.js might be better though as it should highlight the issue:

  it("should show on an formatted error for initial compilation", async () => {
    const compiler = webpack(config);

    new ErrorPlugin("�[0m �[90m 18 |�[39m           �[33mLearn�[39m �[33mReact�[39m�[0m").apply(compiler);

    const devServerOptions = {
      host: "0.0.0.0",
      port,
    };
    const server = new Server(devServerOptions, compiler);

    await new Promise((resolve, reject) => {
      server.listen(devServerOptions.port, devServerOptions.host, (error) => {
        if (error) {
          reject(error);

          return;
        }

        resolve();
      });
    });

    const { page, browser } = await runBrowser();

    await page.goto(`http://localhost:${port}/main`, {
      waitUntil: "networkidle0",
    });

    const pageHtml = await page.evaluate(() => document.body.outerHTML);
    const overlayHandle = await page.$("#webpack-dev-server-client-overlay");
    const overlayFrame = await overlayHandle.contentFrame();
    const overlayHtml = await overlayFrame.evaluate(
      () => document.body.outerHTML
    );

    expect(prettier.format(pageHtml, { parser: "html" })).toMatchSnapshot(
      "page html"
    );
    expect(prettier.format(overlayHtml, { parser: "html" })).toMatchSnapshot(
      "overlay html"
    );

    await browser.close();
    await new Promise((resolve) => {
      server.close(() => {
        resolve();
      });
    });
  });

The produced snapshot:

exports[`overlay should show on an formatted error for initial compilation: overlay html 1`] = `
"<body>
  <div
    id=\\"webpack-dev-server-client-overlay-div\\"
    style=\\"
      position: fixed;
      box-sizing: border-box;
      inset: 0px;
      width: 100vw;
      height: 100vh;
      background-color: rgba(0, 0, 0, 0.85);
      color: rgb(232, 232, 232);
      font-family: Menlo, Consolas, monospace;
      font-size: large;
      padding: 2rem;
      line-height: 1.2;
      white-space: pre-wrap;
      overflow: auto;
    \\"
  >
    <span>Compiled with problems:</span
    ><button
      style=\\"
        background: transparent;
        border: none;
        font-size: 20px;
        font-weight: bold;
        color: white;
        cursor: pointer;
        float: right;
      \\"
    >
      X</button
    ><br /><br />
    <div>
      <span style=\\"color: rgb(227, 96, 73)\\">Error:</span><br /><br />&lt;span
      style=\\"font-weight:normal;opacity:1;color:#transparent;background:#transparent;\\"&gt;
      &lt;span style=\\"color:#6D7891;\\"&gt; 18 |&lt;/span&gt; &lt;span
      style=\\"color:#FFD080;\\"&gt;Learn&lt;/span&gt; &lt;span
      style=\\"color:#FFD080;\\"&gt;React&lt;/span&gt;&lt;/span&gt;<br /><br /><br />
    </div>
  </div>
</body>
"
`;

exports[`overlay should show on an formatted error for initial compilation: page html 1`] = `
"<body>
  <script type=\\"text/javascript\\" charset=\\"utf-8\\" src=\\"/main.js\\"></script>
  <iframe
    id=\\"webpack-dev-server-client-overlay\\"
    src=\\"about:blank\\"
    style=\\"
      position: fixed;
      inset: 0px;
      width: 100vw;
      height: 100vh;
      border: none;
      z-index: 2147483647;
    \\"
  ></iframe>
</body>
"
`;

(Notice how the output from ansiHTML is completely escaped by the usage of document.createTextNode)

@raix raix changed the title Error overlay html encodes the html formatting the error output Error overlay html encodes the ansi html formatting the error output Jul 31, 2021
@raix raix mentioned this issue Jul 31, 2021
Merged
6 tasks
@raix
Copy link
Contributor Author

raix commented Jul 31, 2021

I've added a pull-request for fixing the issue and cover ansi formatting by tests #3579

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

Successfully merging a pull request may close this issue.

fix(overlay): render ansi formattet error messages correctly
5 participants
@raix @snitin315 @erikt9 and others