#44533 closed defect (bug) (fixed)
wp_is_stream wrappers need preg delimiter when quoting
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | 5.1 | Priority: | normal |
| Severity: | normal | Version: | |
| Component: | Filesystem API | Keywords: | needs-unit-tests needs-patch |
| Focuses: | Cc: |
Description
With change r42432, the stream wrappers are now quoted with preg_quote() before being passed into preg_match(). However, the call to preg_match() is using ! as a delimiter. This means that the calls to preg_quote() should also have that delimiter included as the second parameter.
Attachments (2)
Change History (14)
#2
@
7 years ago
- Keywords has-unit-tests added
I decided to go back and add some unit tests for wp_is_stream(). The second patch includes those tests.
This ticket was mentioned in Slack in #core-media by pbiron. View the logs.
7 years ago
#5
@
7 years ago
- Milestone 4.9.8 deleted
- Resolution set to wontfix
- Status changed from new to closed
Thank you for the report and patch, @JPry!
I was thinking about this, but we don't really need to make the change, for two reasons:
!is a special character thatpreg_quote()already escapes.!isn't a character that PHP allows in stream names, so if it shows up in$wrappers, things have gone horribly wrong.
#6
@
7 years ago
Thanks for the explanation @pento
What about the unit tests? Would these still be helpful for wp_is_stream() in general? They weren't particular to the use of preg_quote(), and I don't think there's currently any coverage for wp_is_stream() in core.
#8
@
7 years ago
- Keywords needs-unit-tests needs-patch added; has-patch has-unit-tests removed
- Milestone set to 5.0
- Version trunk deleted
The tests are failing on PHP 5.2:
1) Tests_Functions::test_wp_is_stream with data set #0 ('https://example.com', true)
Failed asserting that false matches expected true.
/home/travis/build/WordPress/wordpress-develop/tests/phpunit/tests/functions.php:1464
https://travis-ci.org/WordPress/wordpress-develop/jobs/405160164
It appears that Travis' PHP 5.2 build doesn't have openssl built in, so https is not a stream that will actually work. 🙂
I would suggest testing for extension_loaded( 'openssl' ), and use that to decide whether a http or https URL should be used.
Same as first patch, but with unit tests added