[wp-trac] [WordPress Trac] #55635: wp_convert_hr_to_bytes() report correct byte sizes for php.ini "shorthand" values

WordPress Trac noreply at wordpress.org
Thu Apr 28 02:01:53 UTC 2022


#55635: wp_convert_hr_to_bytes() report correct byte sizes for php.ini "shorthand"
values
-------------------------------------------------+-------------------------
 Reporter:  dmsnell                              |       Owner:  (none)
     Type:  defect (bug)                         |      Status:  new
 Priority:  normal                               |   Milestone:  Awaiting
                                                 |  Review
Component:  Upload                               |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  needs-unit-tests has-patch dev-      |     Focuses:
  feedback changes-requested                     |
-------------------------------------------------+-------------------------

Comment (by dd32):

 I absolutely hate the idea of writing PHP in the C style, but ultimately
 love the reason for doing so.

 However, I have some concerns, specifically that this is a bit over the
 top for what it's trying to do.

 My notes:
  - `GB_IN_BYTES` can technically be changed, but if someone does that they
 deserve it.
  - The leading whitespace checks aren't working, as it's using single-
 quoted strings, so looking for a literal '\t' 2-character string
  - Isn't written in PHP coding style
  - I don't think it needs to mirror PHP's parsing for a specific config

 [attachment:"55635.3.diff"] is my take on the original issue, I haven't
 run it against the unit tests, but I think it'll resolve the underlying
 intention.
 You can see the evolution in this gist:
 https://gist.github.com/dd32/b6d757bf26f7e417621076fe06925ea5/revisions
 where I took [attachment:"55635.diff"] and slowly migrated it back to PHP,
 before coming to the conclusion that `inval( $s, 0 )` is almost the same,
 albeit with additional binary notation support (which PHP ignores for ini
 settings, but that's fine IMHO)

 It's worth noting, that just because PHP parses it one way, it's not
 necessarily strictly required that `wp_convert_hr_to_bytes()` should
 handle those extreme edge-cases.. but there's some obvious improvements
 that can be made here.

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


More information about the wp-trac mailing list