Skip to content

fix(data-export): name index.php in S3 completion redirects#1529

Merged
rumblefrog merged 1 commit into
mainfrom
fix/export-s3-redirect-post-required
Jul 23, 2026
Merged

fix(data-export): name index.php in S3 completion redirects#1529
rumblefrog merged 1 commit into
mainfrom
fix/export-s3-redirect-post-required

Conversation

@rumblefrog

Copy link
Copy Markdown
Member

Description

Fixes the S3-delivery mode of the owner-only Full data export: pasting a presigned S3 URL and submitting produced a bare "POST required." screen instead of a success/error toast.

web/export.php's two completion redirects (sbpp_export_redirect_success / sbpp_export_redirect_failure) emitted a query-only Location: ?p=admin&c=export&result=... reference. Because the redirect fires from export.php, the browser resolved the empty-path reference against export.php's own path (RFC 3986 §5.2 keeps the base path) and followed it as a GET to export.php?p=admin&c=export..., landing straight in the POST-only method gate at the top of the script. The S3 upload had already run by then, so the operator saw a bogus error after a successful export.

Both arms now name the router entry point explicitly (Location: index.php?p=admin&c=export...), so the redirect resolves to /index.php?p=admin&c=export... and the admin page handler paints the toast.

ZIP mode streams its body to php://output and never redirects, which is exactly why only the S3 path was affected (ZIP downloads worked fine).

Motivation and Context

Operator report: "When I paste a presigned S3 URL into the export, I get a screen 'POST required.'"

The S3 completion path is the one export branch that redirects, and it had no end-to-end coverage (only the uploader class is unit-tested via transport injection), so the bad redirect target slipped through.

How Has This Been Tested?

  • php -l clean on both edited files.
  • Verified the new test's three assertions pass against the edited export.php:
    • no bare Location: ?p= remains,
    • success arm names index.php,
    • error arm names index.php.
  • Confirmed these are the only two Location: redirects in export.php and that every S3 branch (success + all failure arms) routes through the two fixed helpers.

Added testEntryPointRedirectsNameIndexPhpNotBareQuery() to AdminExportPermissionTest.php as a regression guard (static-shape, no DB): fails if a bare Location: ?p= reappears.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

Docs note: this is a redirect-path bugfix that makes the already-documented S3 flow work as written (no change to wire format, delivery modes, or the security model), so data-export.mdx / ARCHITECTURE.md need no update.

The S3 export completion redirects used a query-only `Location: ?p=...`
reference. Firing from web/export.php, the browser resolved it against
export.php's own path (RFC 3986 section 5.2 keeps the base path for an
empty-path reference) and followed it as a GET to
export.php?p=admin&c=export..., landing in the POST-only method gate
("POST required."). The upload had already run, so the operator saw
a bogus error after a successful export.

Name index.php explicitly in both completion arms so the redirect
resolves to the router entry point (/index.php?p=admin&c=export...) and
the admin page handler paints the success/error toast. ZIP mode streams
its body and never redirects, which is why only the S3 path broke.

Adds testEntryPointRedirectsNameIndexPhpNotBareQuery() as a regression
guard: fails if a bare `Location: ?p=` reappears and asserts both arms
name index.php.
Copilot AI review requested due to automatic review settings July 23, 2026 05:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@rumblefrog
rumblefrog added this pull request to the merge queue Jul 23, 2026
Merged via the queue into main with commit de4526f Jul 23, 2026
5 of 6 checks passed
@rumblefrog
rumblefrog deleted the fix/export-s3-redirect-post-required branch July 23, 2026 06:32
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants