Skip to content

fix: do not escape HTML #2007

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

Merged
merged 5 commits into from
Mar 23, 2023
Merged

fix: do not escape HTML #2007

merged 5 commits into from
Mar 23, 2023

Conversation

LekoArts
Copy link

@LekoArts LekoArts commented Mar 22, 2023

Summary

When setPageProp is called it in turn calls transformData which pushes to the dataTransforms array. Inside the shared edge util it gets accessed:

https://github.com/netlify/next-runtime/blob/fb93b5469c570616134d53c10fbed1f7ef78e334/packages/runtime/src/templates/edge-shared/utils.ts#L203-L225

The HTML rewriter is a Deno port of https://developers.cloudflare.com/workers/runtime-apis/html-rewriter/.

By default it escapes HTML, but there is a html setting: https://developers.cloudflare.com/workers/runtime-apis/html-rewriter/#global-types

So we can just set this to true and fix the issue of incorrectly escaping raw HTML while rewriting the HTML.

Test plan

See that the changed e2e test still passes.

Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal

Fixes #1629

Standard checks:

  • Check the Deploy Preview's Demo site for your PR's functionality

🧪 Once merged, make sure to update the version if needed and that it was published correctly.

@LekoArts LekoArts requested a review from a team March 22, 2023 16:02
@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!

Name Link
🔨 Latest commit 06df12f
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-nx-monorepo-demo/deploys/641c0ad8cecbe30008fce68a
😎 Deploy Preview https://deploy-preview-2007--netlify-plugin-nextjs-nx-monorepo-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!

Name Link
🔨 Latest commit 06df12f
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-static-root-demo/deploys/641c0ad84f4716000818f6a0
😎 Deploy Preview https://deploy-preview-2007--netlify-plugin-nextjs-static-root-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions github-actions bot added the type: bug code to address defects in shipped code label Mar 22, 2023
@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for netlify-plugin-nextjs-demo ready!

Name Link
🔨 Latest commit 06df12f
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-demo/deploys/641c0ad8e0e51d00087d3927
😎 Deploy Preview https://deploy-preview-2007--netlify-plugin-nextjs-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for next-plugin-edge-middleware ready!

Name Link
🔨 Latest commit 06df12f
🔍 Latest deploy log https://app.netlify.com/sites/next-plugin-edge-middleware/deploys/641c0ad8b5086b00085faa72
😎 Deploy Preview https://deploy-preview-2007--next-plugin-edge-middleware.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for nextjs-plugin-custom-routes-demo ready!

Name Link
🔨 Latest commit 06df12f
🔍 Latest deploy log https://app.netlify.com/sites/nextjs-plugin-custom-routes-demo/deploys/641c0ad8c0d298000786d0e9
😎 Deploy Preview https://deploy-preview-2007--nextjs-plugin-custom-routes-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for next-plugin-canary ready!

Name Link
🔨 Latest commit 06df12f
🔍 Latest deploy log https://app.netlify.com/sites/next-plugin-canary/deploys/641c0ad84cc37d000842123b
😎 Deploy Preview https://deploy-preview-2007--next-plugin-canary.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for netlify-plugin-nextjs-export-demo ready!

Name Link
🔨 Latest commit 06df12f
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-export-demo/deploys/641c0ad816f93d0008616793
😎 Deploy Preview https://deploy-preview-2007--netlify-plugin-nextjs-export-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for next-i18next-demo ready!

Name Link
🔨 Latest commit 06df12f
🔍 Latest deploy log https://app.netlify.com/sites/next-i18next-demo/deploys/641c0ad8e0e51d00087d3922
😎 Deploy Preview https://deploy-preview-2007--next-i18next-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!

Name Link
🔨 Latest commit 06df12f
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-next-auth-demo/deploys/641c0ad89bbf300008b883cb
😎 Deploy Preview https://deploy-preview-2007--netlify-plugin-nextjs-next-auth-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link

@nickytonline nickytonline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a suggestion, but this works great! :shipit:

@@ -24,7 +24,7 @@ export async function middleware(req: NextRequest) {
if (pathname.startsWith('/static')) {
// Unlike NextResponse.next(), this actually sends the request to the origin
const res = await request.next()
const message = `This was static but has been transformed in ${req.geo?.city}`
const message = `This was static & old but has been transformed in ${req.geo?.city}`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are great. I wonder if it makes sense to test that & would render as the text &? The only use case for that that I can think of is if a code example was in a prop being set. Maybe not worth it. 🤔

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@LekoArts LekoArts merged commit d4cd121 into main Mar 23, 2023
@LekoArts LekoArts deleted the fix-ampersand branch March 23, 2023 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Using setPageProps causes rendering mismatch when using with ampersands
3 participants