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.

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

__NEXT_DATA__ is not escaped in amp. #11159

Closed
dan-weaver opened this issue Mar 18, 2020 · 2 comments · Fixed by ampproject/amp-toolbox#649 or #11168
Closed

__NEXT_DATA__ is not escaped in amp. #11159

dan-weaver opened this issue Mar 18, 2020 · 2 comments · Fixed by ampproject/amp-toolbox#649 or #11168
Labels
good first issue Easy to fix issues, good for newcomers

Comments

@dan-weaver
Copy link
Contributor

dan-weaver commented Mar 18, 2020

Bug report

AMP pages do not render NEXT_DATA correctly. Results in white screen.

Describe the bug

if NEXT_DATA contains any markup within the object, that markup is "optimized" and results in unescaped characters in some scenarios. The page in the example below will not render. Disabling amp optimization fixes the issue.

To Reproduce

I made a small repro here: https://github.com/dan-weaver/next-amp-bug. I realize this is an edge case but the data here is valid json and the contents of the string is valid html!

  1. Clone https://github.com/dan-weaver/next-amp-bug
  2. run yarn dev
  3. Goto `http://localhost:3000. The page will work fine. (I removed the css nbd).
  4. Goto http://localhost:3000?amp=1. The page will display a white screen.
  5. Compare the contents of the __NEXT_DATA__ scripts on both url's.
  6. Uncommenting this (https://github.com/zeit/next.js/blob/canary/packages/next/next-server/server/render.tsx#L746)[line] appears to fix the issue, but it obviously will break amp ooptimizations.

Expected behavior

NEXT_DATA should be rendered the same in both amp and non amp.

Wondering what the best solution is. I can't quite understand the intention of the source code I pointed to above with AMP_TARGET etc. Maybe providing an option to not use optimization would be a good stopgap?

@timneutkens timneutkens added good first issue Easy to fix issues, good for newcomers help wanted labels Mar 18, 2020
@sebastianbenz
Copy link
Contributor

The latest AMP Optimizer release minifies inline JSON, but does not escape angle brackets. This results in the browser not being able to correctly parse the inline JSON. Will fix this in AMP Optimizer.

sebastianbenz added a commit to ampproject/amp-toolbox that referenced this issue Mar 18, 2020
sebastianbenz added a commit to ampproject/amp-toolbox that referenced this issue Mar 18, 2020
sebastianbenz added a commit to ampproject/amp-toolbox that referenced this issue Mar 18, 2020
sebastianbenz added a commit to sebastianbenz/next.js that referenced this issue Mar 18, 2020
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Easy to fix issues, good for newcomers
Projects
None yet
4 participants