Make WordPress Core

Opened 7 years ago

Last modified 7 years ago

#41353 new defect (bug)

Async upload breaks when there are special characters into file name

Reported by: bhargavbhandari90's profile bhargavbhandari90 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.8
Component: Media Keywords: has-patch dev-feedback
Focuses: Cc:

Description

After crunching the image, thumbnail breaks for the image.

Attachments (2)

пёсики-песочница^610680.jpg (37.2 KB) - added by bhargavbhandari90 7 years ago.
Here is the file. Try to upload this and check after crunching.
41353.patch (729 bytes) - added by bhargavbhandari90 7 years ago.
This will be the solution

Download all attachments as: .zip

Change History (8)

@bhargavbhandari90
7 years ago

Here is the file. Try to upload this and check after crunching.

@bhargavbhandari90
7 years ago

This will be the solution

#1 @bhargavbhandari90
7 years ago

  • Keywords needs-testing has-patch added

#2 follow-up: @MaximeCulea
7 years ago

Hi @bhargavbhandari90, thx for the report.
I wonder if the issue is more "elaborate".
To me it's the sanitize itself that should be done upstream to handle special caracters, like reported in #22363.
What do you think ?

#3 in reply to: ↑ 2 @bhargavbhandari90
7 years ago

Replying to MaximeCulea:

Hi @bhargavbhandari90, thx for the report.
I wonder if the issue is more "elaborate".
To me it's the sanitize itself that should be done upstream to handle special caracters, like reported in #22363.
What do you think ?

Thanks for pointing out this.

Yes it makes sense. But why I am suggesting the current solution is because the image was not broken into the edit image section.

So I checked the code for that into "wp-admin/includes/media.php:2740" and at there, I found that wp used set_url_scheme.

So for now I thought to handle this situation by the same method which is used into that edit media section.

Give me your thought for this.

Last edited 7 years ago by bhargavbhandari90 (previous) (diff)

#4 @MaximeCulea
7 years ago

  • Focuses administration removed
  • Keywords needs-testing removed

@bhargavbhandari90,

Even if the issue could be handle at upstream level, I agree with you that the escaping method here is not the best and cause an unwanted error.

As you pointed, you definitely need to use set_url_scheme and you patch seems fine.

I just tested it and it works like a charm, so more need for needs-testing.
I also took off the administration tag, as it's not into the administration focus.

#5 follow-up: @SergeyBiryukov
7 years ago

Related: #15955

#6 in reply to: ↑ 5 @bhargavbhandari90
7 years ago

  • Keywords dev-feedback added

Replying to SergeyBiryukov:

Related: #15955

Thank you @SergeyBiryukov for pointing out the related issue. That will solve the problem.
But currently only problem is during the async-upload.

The image is showing all other places except in async-upload.

And I think this solution will also adapt the behavior of #15955

Note: See TracTickets for help on using tickets.