Skip to content

fix: External preview URLs #7256

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 2 commits into from
Jun 2, 2025
Merged

Conversation

lukasbestle
Copy link
Member

@lukasbestle lukasbestle commented Jun 1, 2025

🚨 Merge first

Description

Summary of changes

  • Use URL instead of relative URI for preview tokens
  • Don’t escape slashes in JSON
  • Support external preview targets

Reasoning

  • We need to use the full URL instead of the relative URI in all cases (even internal previews) to make the behavior consistent and avoid generating different preview tokens for the same target page (one with and one without full URL) – this would mean that validating the token would need to check for both, which would be cumbersome and complex.
  • Using the full URL has the disadvantage that the token validation may fail if the base URL is detected differently on rendering the preview than when generating the preview URL. This could in theory happen in exotic setups of the url option, but is not very likely in practice. If it happens, it can be fixed by fixing the custom url setup. This was the reason why I originally decided to use the relative path (which made external previews impossible), but for the headless use-case we need the external preview support and so this solution seems to be the most solid overall.

Changelog

Fixes since v5 beta

Breaking changes since v5 beta

  • Change the underlying logic for preview tokens yet again (breaking preview URLs created in previous v5 pre-releases).

Docs

None

Ready?

  • In-code documentation (wherever needed)
  • Unit tests for fixed bug/feature
  • Tests and CI checks all pass

For review team

  • Add changes & docs to release notes draft in Notion

URIs contain a lot of slashes. We don’t need to escape them here as the JSON object is immediately passed through `hash_hmac()` afterwards. PHP only escapes slashes by default for cases where the JSON is placed inside a `<script>` tag.
@lukasbestle lukasbestle added this to the 5.0.0-rc.3 milestone Jun 1, 2025
@lukasbestle lukasbestle requested a review from a team June 1, 2025 21:08
@lukasbestle lukasbestle self-assigned this Jun 1, 2025
@lukasbestle lukasbestle linked an issue Jun 1, 2025 that may be closed by this pull request
Allows to generate preview tokens for external preview URLs. See reasoning in #7240 (comment) and following comments.

Fixes #7240
@lukasbestle lukasbestle force-pushed the v5/fix/external-preview branch from 913823e to 8f513a5 Compare June 2, 2025 05:44
@lukasbestle lukasbestle marked this pull request as ready for review June 2, 2025 05:45
Base automatically changed from v5/fix/uri-slash to v5/develop June 2, 2025 09:02
@bastianallgeier bastianallgeier merged commit bb21888 into v5/develop Jun 2, 2025
12 checks passed
@bastianallgeier bastianallgeier deleted the v5/fix/external-preview branch June 2, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[V5] Preview-URL for headless setup
2 participants