[wp-trac] [WordPress Trac] #50855: sanitize_file_name function not working as expected if there is '%20' in filename

[WordPress Trac]

#50855: sanitize_file_name function not working as expected if there is '%20' in
filename
-------------------------------------------------+-------------------------
 Reporter:  dishitpala                           |       Owner:  audrasjb
     Type:  defect (bug)                         |      Status:  accepted
 Priority:  normal                               |   Milestone:  5.9
Component:  Formatting                           |     Version:  5.4.2
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-screenshots has-unit-  |     Focuses:
  tests dev-feedback                             |
-------------------------------------------------+-------------------------
Changes (by hellofromTonya):

 * keywords:  has-patch has-screenshots has-unit-tests => has-patch has-
     screenshots has-unit-tests dev-feedback


Comment:

 [29290] #16630 added the logic to search for both `%20` and `+` and then
 replace them with `+`.

 But then [35122] #16226 added the `%` and `+` as special characters which
 replaces each with an empty string (strips them out).

 Notice the timing of these changes: [29290] happened first and then
 [35122].

 [attachment: "sanitize-file-name.patch"] [attachment:"50855.diff"]
 [attachment:"50855.2.diff"] move both the `%20` and `+` to `-` replacers
 above the special characters replacers. ''That could be a breaking
 change''. Why?

 * The `+` character: since [35122] (in 10/13/2015), the `+` character has
 been removed (i.e. replaced with an empty string). Moving it above the
 special characters means it no longer is stripped out but instead is
 replaced with a `-`.
 * The `%` character: since [29290] (in 07/24/2014), the `%` character was
 stripped leaving the `20`. Notice
 [https://core.trac.wordpress.org/ticket/16226#comment:15 Pippin identified
 that behavior here].

 Given the history, I'm seeking more input on:

 * Should the `+` character be stripped (as special character) or replaced
 with a `-`? If special character, then is the `+` in this code needed?
 {{{#!php
 $filename = str_replace( array( '%20', '+' ), '-', $filename );
 }}}

 * Is leaving `20` (stripping the `%`) by design? If no, do you see any
 issues replacing `%20` with `-`?

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/50855#comment:10>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform