WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 17 months ago

Last modified 17 months ago

#40861 closed defect (bug) (fixed)

REST API saves attachments with absolute path for `_wp_attached_file` on Windows platforms

Reported by: redcastor Owned by: SergeyBiryukov
Milestone: 4.9.8 Priority: normal
Severity: normal Version: 4.6
Component: REST API Keywords: has-patch needs-unit-tests
Focuses: Cc:
PR Number:

Description

On windows, create media with rest api insert the meta "_wp_attached_file" not correctly.
It save the absolute path and not the relative path.

the function "wp_slash" add backslash in windows file path.

file: class-rest-attachements-controller.php
line: 150

<?php
$id = wp_insert_post( wp_slash( (array) $attachment ), true );

Attachments (2)

40861.patch (480 bytes) - added by ocean90 22 months ago.
40861.2.patch (983 bytes) - added by ocean90 22 months ago.

Download all attachments as: .zip

Change History (16)

This ticket was mentioned in Slack in #core by bobbingwide. View the logs.


22 months ago

#2 @bobbingwide
22 months ago

This bug causes a problem with Gutenberg. See https://github.com/WordPress/gutenberg/issues/5332

#3 @SergeyBiryukov
22 months ago

  • Milestone changed from Awaiting Review to 5.0

#4 @ocean90
22 months ago

  • Keywords needs-patch added
  • Summary changed from Rest Api create media to REST API saves attachments with absolute path for `_wp_attached_file` on Windows platforms
  • Version changed from 4.7.5 to 4.6

Looks like _wp_relative_upload_path() is missing here.

Last edited 22 months ago by ocean90 (previous) (diff)

@ocean90
22 months ago

@ocean90
22 months ago

#5 @ocean90
22 months ago

  • Keywords has-patch needs-unit-tests added; needs-patch removed

Attached 40861.patch and 40861.2.patch.

Since wp_insert_post() expects slashed data we should unslash the file argument as well, done in 40861.patch.

In 40861.2.patch I switched to wp_insert_attachment() so the file argument doesn't get slashed in the first place.

#6 follow-up: @bobbingwide
22 months ago

The second patch makes it a lot more obvious that it's difficult to use the REST API to create an attachment which is actually attached to something! Is that one of the reasons for the 'rest_insert_attachment' action hook?

#7 in reply to: ↑ 6 @ocean90
22 months ago

Replying to bobbingwide:

The second patch makes it a lot more obvious that it's difficult to use the REST API to create an attachment which is actually attached to something!

Not sure I'm following. wp_insert_attachment() is a simple wrapper for wp_insert_post() without any additional filters. What's the issue you're seeing here?

#8 @danielbachhuber
18 months ago

  • Milestone changed from 5.0 to 4.9.7

Let's land this in 4.9.7 so we can see this sooner rather than later.

#9 @ocean90
17 months ago

  • Milestone changed from 4.9.7 to 4.9.8

4.9.7 has been released, moving to next milestone.

This ticket was mentioned in Slack in #core by pbiron. View the logs.


17 months ago

This ticket was mentioned in Slack in #core by pbiron. View the logs.


17 months ago

This ticket was mentioned in Slack in #core by jon_bossenger. View the logs.


17 months ago

#13 @SergeyBiryukov
17 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 43529:

REST API: Ensure attachments created with WP_REST_Attachments_Controller::create_item() on Windows have a relative path.

Props ocean90, SergeyBiryukov, redcastor.
Fixes #40861.

#14 @SergeyBiryukov
17 months ago

In 43530:

REST API: Ensure attachments created with WP_REST_Attachments_Controller::create_item() on Windows have a relative path.

Props ocean90, SergeyBiryukov, redcastor.
Merges [43529] to the 4.9 branch.
Fixes #40861.

Note: See TracTickets for help on using tickets.