Skip to content

Fix/threads jsonld html script escape#2983

Merged
eldadfux merged 2 commits into
mainfrom
fix/threads-jsonld-html-script-escape
May 7, 2026
Merged

Fix/threads jsonld html script escape#2983
eldadfux merged 2 commits into
mainfrom
fix/threads-jsonld-html-script-escape

Conversation

@eldadfux
Copy link
Copy Markdown
Member

@eldadfux eldadfux commented May 7, 2026

What does this PR do?

(Provide a description of what this PR does.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

eldadfux and others added 2 commits May 6, 2026 23:23
Thread JSON-LD can include code samples with </script>. Escaping < as
\u003c in serialized JSON prevents the HTML parser from closing script
tags early in both the ld+json block and SvelteKit route data.

Co-authored-by: Cursor <cursoragent@cursor.com>
…solution

- Added support for dynamic hero titles in the marketing page by resolving query parameters.
- Introduced new constants for default hero attributes and integrated them into the page logic.
- Updated the layout to ensure the document title reflects the hero heading accurately.

This improves the SEO and user experience by providing relevant titles based on user context.
@appwrite
Copy link
Copy Markdown

appwrite Bot commented May 7, 2026

Appwrite Website

Project ID: 69d7efb00023389e8d27

Sites (1)
Site Status Logs Preview QR
 website
69d7f2670014e24571ca
Failed Failed View Logs Preview URL QR Code

Tip

GraphQL API works alongside REST and WebSocket protocols

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 7, 2026

Greptile Summary

This PR fixes a script-injection risk where user-supplied content containing </script> could prematurely close an inline JSON-LD <script> tag on thread pages. It also includes an unrelated homepage change that derives the <title> dynamically from a Statsig hero-variant A/B test.

  • metadata.ts: Extracts escapeJsonLtForHtmlScript as a shared helper; createDiscussionForumPageSchema now pre-escapes its JSON output so the string is safe when SvelteKit serializes it into the page's devalue inline script; logo URL updated from .avif to .png.
  • (marketing)/+page.svelte: Adds Statsig hero-variant imports and derives homepageDocumentTitle from the resolved hero title — independent of the JSON-LD fix.
  • threads/[id]/+page.svelte: Cosmetic CSS: header align-items changed from center to start.

Confidence Score: 4/5

The JSON-LD escaping fix is correct and the HTML output is safe; the only real concern is a design inconsistency in how escaping is applied.

The escaping logic works correctly end-to-end: createDiscussionForumPageSchema pre-escapes < so the string is safe in SvelteKit's page-data serialization, and the second escape pass in getInlinedScriptTag is harmless (no-op). The inconsistency — this function pre-escaping while every other schema function returns raw JSON — is a maintenance concern rather than a functional defect. The unrelated homepage title change is straightforward and low-risk.

src/lib/utils/metadata.ts warrants a second look regarding the escaping contract between createDiscussionForumPageSchema and getInlinedScriptTag; the marketing page change in src/routes/(marketing)/+page.svelte is unrelated to the stated fix and should be verified independently.

Important Files Changed

Filename Overview
src/lib/utils/metadata.ts Extracts escapeJsonLtForHtmlScript as a reusable helper; createDiscussionForumPageSchema now pre-escapes its output (making the subsequent escape in getInlinedScriptTag a no-op); logo URL changed from .avif to .png
src/routes/(marketing)/+page.svelte Adds hero-variant-driven dynamic homepage <title> via Statsig A/B testing helpers — appears unrelated to the JSON-LD escaping fix described in the PR
src/routes/threads/[id]/+page.svelte Cosmetic CSS change: .header alignment changed from center to start; no logic changes

Comments Outside Diff (1)

  1. src/routes/(marketing)/+page.svelte, line 1-53 (link)

    P2 Unrelated change bundled into a JSON-LD escaping fix

    The additions in this file — importing building, page, Statsig constants, resolveHeroQueryOverrides, and deriving homepageDocumentTitle from hero variant data — are independent of the JSON-LD </script> escaping described in the PR title. Mixing two unrelated features in one PR makes it harder to bisect regressions. If these changes are intentional, a brief note in the PR description explaining the second change would help reviewers.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "feat(marketing): enhance hero section wi..." | Re-trigger Greptile

Comment thread src/lib/utils/metadata.ts
};

return JSON.stringify(graph);
return escapeJsonLtForHtmlScript(JSON.stringify(graph));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Pre-escaping creates redundant second pass in getInlinedScriptTag

createDiscussionForumPageSchema now returns a pre-escaped string (all < replaced with \u003c), but its output is then passed to getInlinedScriptTag, which calls escapeJsonLtForHtmlScript a second time. The second pass is a no-op because no < characters remain, so the HTML output is correct. However, every other schema function (organizationJsonSchema, softwareAppSchema, createPostSchema, etc.) returns raw JSON and relies on getInlinedScriptTag to do the escaping — createDiscussionForumPageSchema is now the only outlier.

If the motivation is to protect structuredDataJsonLd when SvelteKit serializes it into the page's devalue inline script (a separate <script> from the JSON-LD tag), that is a valid concern — but it may be worth a comment at the call site in +page.server.ts explaining why this function pre-escapes while others do not, so future callers of getInlinedScriptTag don't expect all inputs to behave the same way.

@eldadfux eldadfux merged commit 8bc5827 into main May 7, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant