Skip to content

fix magic url base to not redirect keyless links to expired page#2909

Merged
corsacca merged 1 commit intodevelopfrom
magit-link-without-keys
Apr 13, 2026
Merged

fix magic url base to not redirect keyless links to expired page#2909
corsacca merged 1 commit intodevelopfrom
magit-link-without-keys

Conversation

@corsacca
Copy link
Copy Markdown
Member

restore public_key guard removed in befbd76 so redirect-only magic links (e.g. newest/map) are not killed before child constructors run

restore public_key guard removed in befbd76 so redirect-only magic links (e.g. newest/map) are not killed before child constructors run
@github-actions
Copy link
Copy Markdown

Code Review

No High or Medium severity issues found.

Analysis

The fix is correct. determine_post_id() (magic-url-class.php:407-423) only sets post_id when public_key is non-empty — for keyless magic links it's a no-op. The old guard at magic-url-base.php:51 (before this PR) then unconditionally redirected any link without a resolved post_id to the expired page, which incorrectly killed legitimate keyless links (e.g. redirect-only, map) before their child constructors could run.

The restored condition \!empty( $this->parts['public_key'] ) && empty( $this->parts['post_id'] ) correctly limits the expired-redirect to links that do carry a key (and therefore are supposed to resolve to a post/user) but failed to find one — the genuine expired/invalid-key case.

No security concern: a request without a public_key still must match a registered root+type pair (enforced by parse_url_parts()), the type-match check at line 46-48 still runs, and the dt_magic_link_continue filter at line 57-59 still provides a veto point. The register_url method already handles the keyless case at line 306 (if ( empty( $parts['public_key'] ) )), confirming this is an established supported pattern.

Ready to merge.

@corsacca corsacca merged commit 7b4f062 into develop Apr 13, 2026
5 checks passed
@corsacca corsacca deleted the magit-link-without-keys branch April 13, 2026 10:03
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