Skip to content

Fix remote featured images with query-string URLs failing import - #8063

Merged
donnapep merged 5 commits into
trunkfrom
fix/query-string-remote-image-import
Jul 6, 2026
Merged

donnapep merged 5 commits into
trunkfrom
fix/query-string-remote-image-import

Conversation

@claude

@claude claude Bot commented Jul 6, 2026 •

Copy link
Copy Markdown

Summary

  • basename( $external_url ) on a URL like https://i0.wp.com/example.com/image.png?fit=2560%2C1440&ssl=1 returns image.png?fit=2560%2C1440&ssl=1 — the query string becomes part of the filename, so wp_upload_bits() rejects it with "Sorry, you are not allowed to upload this file type." Every remote featured image referenced with a query-string URL (Photon/Jetpack, signed CDN URLs, etc.) fails to attach.
  • Fixed by deriving the filename from the URL path only, via basename( (string) wp_parse_url( $external_url, PHP_URL_PATH ) ). The cast avoids a PHP 8.1+ basename( null ) deprecation for path-less URLs; the subsequent wp_check_filetype_and_ext() still validates the actual file contents.
  • Added a regression test covering a Photon/Jetpack-style query-string URL.

Fixes #8059

Test plan

  • Import a CSV with a remote featured image URL that includes a query string (e.g. a Jetpack Photon URL like https://i0.wp.com/example.com/image.png?fit=800%2C600&ssl=1) and confirm the image attaches to the imported lesson/course without warnings.

🤖 Generated with Claude Code

github-actions Bot and others added 2 commits July 6, 2026 14:51
basename() on a URL with a query string includes the query string in the
filename, causing wp_upload_bits() to reject the file as an unknown type.
Strip the query string by deriving the filename from the URL path only.
Also fix pre-existing lint violations in the modified file.

Fixes #8059

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous commit accidentally corrupted the left curly quote character
(U+201C) to a right curly quote (U+201D) during the parameter rename.
Restore the original bytes by re-applying only the identifier rename.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude claude Bot added this to the 4.26.2 milestone Jul 6, 2026
@donnapep donnapep self-assigned this Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026 •

Copy link
Copy Markdown
Contributor

WordPress Playground Preview

The changes in this pull request can previewed and tested using a WordPress Playground instance.

Open WordPress Playground Preview

donnapep and others added 3 commits July 6, 2026 11:00
Cast the parsed URL path before basename() to avoid a PHP 8.1+
deprecation when the URL has no path, and drop the extensionless
'import-<md5>' fallback: it produced a filename wp_upload_bits() would
reject with the same "not allowed file type" error, so an empty
filename now fails through naturally as a per-row warning.

Use assertIsInt() in the query-string test for a clearer failure
message and to drop the print_r() debug call flagged by PHPCS.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Restore replace_curly_quotes and split_list_safely to their trunk form
so this PR touches only the attachment filename fix. The two PHPCS
findings this re-introduces (function-keyword spacing, "string" param
name) are pre-existing on trunk and sit outside this PR's diff, so the
changed-line CI linter does not flag them.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Assert only that a query-string URL yields an attachment id — the
behavior this fix restores. Post-type and source-key-meta checks cover
pre-existing behavior already exercised by neighbouring attachment tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

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.

Pull request overview

Fixes Sensei’s data-port remote featured-image import when the source URL includes a query string (e.g. Jetpack Photon URLs), by ensuring the uploaded filename is derived from the URL path instead of including the query string (which breaks wp_upload_bits() extension checks).

Changes:

  • Update Sensei_Data_Port_Utilities::create_attachment_from_url() to derive the upload filename from wp_parse_url( ..., PHP_URL_PATH ).
  • Add a unit test covering a query-string image URL import flow.
  • Add a changelog entry documenting the fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
includes/data-port/class-sensei-data-port-utilities.php Adjusts filename derivation for remote attachment uploads to avoid query strings breaking filetype checks.
tests/unit-tests/data-port/test-class-sensei-data-port-utilites.php Adds regression coverage for query-string URLs in get_attachment_from_source().
changelog/fix-query-string-remote-image-import Documents the user-facing fix as a patch “fixed” entry.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread includes/data-port/class-sensei-data-port-utilities.php
@donnapep donnapep removed their assignment Jul 6, 2026
@donnapep
donnapep merged commit 8ebfdb5 into trunk Jul 6, 2026
25 checks passed
@donnapep
donnapep deleted the fix/query-string-remote-image-import branch July 6, 2026 15:33
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.

Remote featured images with query-string URLs fail import

2 participants