#42439 closed defect (bug) (fixed)
Update random_compat external library for PHP 7 linting failure
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 4.9.2 | Priority: | normal |
| Severity: | normal | Version: | 4.9 |
| Component: | External Libraries | Keywords: | fixed-major |
| Focuses: | Cc: |
Description
Currently the random_compat library included with WordPress core is the only part that fails against PHP 7 linting. As a tech at WP Engine I've noticed that our customers run into problems when pushing to their remote repositories, so I did a bit of investigation and it looks like the external library included was updated to correct this as of October 2016 as commented here: https://core.trac.wordpress.org/ticket/35665#comment:6
I've created the attached diff and it passes all of the unit tests included with core. I've also verified that it lints correctly via the following command.
for file in $(find . -type f -name "*.php"); do php -l $file; done | grep -v "No syntax errors"
#35665 - Previously updated to version 1.2.1 in ticket #35665
Attachments (1)
Change History (16)
#2
@
8 years ago
Diffing the versions in github looks sane, as does this patch.
It's always been expected that these files would not parse under PHP7, as they're not loaded there. Unfortunately it seems that WPEngine blocks deploying these files, causing issues (IMHO, it's WPE's responsibility to allow them to be deployed).
While I have no issue updating these, as we're so close to the 4.9 release, I'm tempted to leave it until 4.9.1 or 5.0 - even though I don't expect it'll cause any breakage.
#3
@
8 years ago
- Milestone changed from Awaiting Review to 4.9.1
Okay, lets get this in 4.9.1 - I'll review once 4.9 is out. At the minimum we'll remove the PHP7 parse errors from 4.9.1 if the changes upstream are larger than expected.
#7
@
8 years ago
- Owner set to dd32
- Resolution set to fixed
- Status changed from new to closed
In 42344:
#8
@
8 years ago
Did a scan to see who was using this. Doesn't look (to me) like anything SHOULD break.
Calling random_int()
67 matching plugins
Matches Plugin Active installs
======= ====== ===============
1 miniorange-login-with-eve-online-google-facebook 200+
1 ilmomasiina-event-manager 0+
1 xcloner-backup-and-restore 70,000+
2 kocuj-sitemap 2,000+
2 wp-chatbull 60+
1 next-active-directory-integration 2,000+
1 wp-concours 300+
1 advanced-dynamic-pricing-for-woocommerce 0+
1 cart-recovery 200+
1 mazen-seo-connector 10+
1 hacklog-remote-image-autosave 3,000+
1 rocket24-analytics 0+
1 perfectdashboard 900+
2 pwaplusphp 2,000+
2 testimonial-master 50+
1 better-wp-security 800,000+
1 pay-post-by-sms 0+
1 sunny 1,000+
1 apocalypse-meow 1,000+
1 fomo-payment-gateway-for-woocommerce 20+
2 business-master 10+
2 random-user-ids 20+
1 ing-psp 200+
2 mansplainer 0+
1 total-security 2,000+
2 automatic-comment-scheduler 80+
1 beeketing-for-woocommerce 3,000+
2 wp-pgp-encrypted-emails 200+
1 google-maps 50,000+
1 live-weather-station 3,000+
2 quiz-master-next 10,000+
1 woo-payment-highway 0+
1 reservation-engine 10+
2 jekyll-exporter 800+
3 formlift 300+
1 woo-ideal-gateway 100+
4 cidram 200+
1 s2member 30,000+
1 questionnaire 500+
3 mg-member-control 0+
2 woocommerce-germanized 30,000+
1 better-coupon-box 200+
1 civic-sip 30+
1 user-registration 700+
1 custom-tabs-shortcodes 0+
1 translation-connectors 0+
1 look-see-security-scanner 1,000+
2 jetpack 4,000,000+
1 sales-pop 900+
1 unc-gallery 10+
1 everest-review-lite 0+
2 quote-master 1,000+
1 salon-booking-system 3,000+
1 frontend-dashboard-custom-post 0+
14 math-quiz 500+
1 setka-editor 4,000+
3 wordfence 2,000,000+
1 wp-demo-buddy 0+
1 awesome-support 6,000+
1 erp 4,000+
1 well-handled 60+
2 woorewards 10+
1 admin-menu 300+
1 axis-subscriptions 30+
1 convertiser-widgets 50+
2 wptimetoread 0+
1 upstream 900+
calling random_bytes()
241 matching plugins
Matches Plugin Active installs
======= ====== ===============
5 wp-rekogni 0+
2 wp-native-php-sessions 10,000+
1 eduadmin-booking 10+
2 miniorange-login-with-eve-online-google-facebook 200+
1 wpx-maintenance-pro-light 20+
1 xcloner-backup-and-restore 70,000+
1 social-nation-itsme-oauth-login-multibutton 0+
1 employee-directory 400+
1 lifterlms 6,000+
2 wp-meta-data-filter-and-taxonomy-filter 10,000+
1 opal-hotel-room-booking 800+
4 joebooking 300+
3 wp-chatbull 60+
5 next-active-directory-integration 2,000+
1 wp-travel 500+
2 wp-hr-manager 10+
1 branding 200+
37 easy-affiliate-cloaker 10+
8 logy 10+
2 webxpay-payment-gateway-for-woocommerce 40+
1 yith-woocommerce-request-a-quote 10,000+
1 leaflet-maps-marker 40,000+
2 trusona 300+
8 premium-seo-pack-light-version 500+
8 jannes-mannes-social-media-auto-publisher 0+
5 import-youtube-videos-as-wp-post 900+
1 learnpress 30,000+
5 ga-experiments-plus-dev-edition 10+
5 static-html-output-plugin 6,000+
1 liveeditor 100+
5 do-spaces-sync 0+
5 dxw-members-only 0+
1 deemly 0+
1 mazen-seo-connector 10+
2 ssh-sftp-updater-support 40,000+
1 idea-board 10+
2 wp-splashing-images 50+
3 wpx-server-light 30+
5 backup-wd 6,000+
2 mucash-micro-payments 10+
1 site-reviews 1,000+
5 wp-stateless 1,000+
39 mpl-publisher 100+
3 wp-otp 80+
5 download-s3-content 0+
5 cf7-spreadsheets 20+
2 php-compatibility-checker 40,000+
38 rocket24-analytics 0+
33 perfectdashboard 900+
5 protect-wp-videos 0+
1 allwebmenus-wordpress-menu-plugin 200+
8 yith-woocommerce-social-login 10,000+
2 easy-digital-downloads 60,000+
5 ider-login 0+
8 join-us 0+
2 iwp-client 400,000+
5 intelligence 80+
2 jigoshop-ecommerce 300+
1 solidres 200+
8 auto-post-woocommerce-products 0+
1 wp-ticket 500+
1 extranet 100+
1 participants-database 10,000+
34 wp-blade-engine 0+
1 campus-directory 100+
5 wp-consent-receipt 0+
2 backup-amazon-s3 1,000+
47 cartrabbit 0+
1 wp-ldap
WordPress.org: Please note that this content has been truncated for display.
#9
@
8 years ago
I'm a plugin author mentioned in the post above.
So does upgrading this library affect us at all, do we have to make any changes?
#10
@
8 years ago
Plugins calling random_int() and random_bytes() should be fine with the changes made here, I'm not really sure why the plugins list was posted :)
Something I am going to note here, is that [42130] Updated Random_Compat from 1.2.1 to 2.0.11, which in the process dropped the OpenSSL provider for WordPress 5.0.
It's possible that the loss of OpenSSL here could be an issue for some environments, notably locked down linux environments and older Windows systems off the top of my head.
I'm contemplating if we shouldn't add the OpenSSL Handler back in, it'll mean we diverge from upstream, but might be the safest option overall for WordPress.
#11
follow-up:
↓ 12
@
8 years ago
@dd32 It was asked in Slack to see who was using it and if they'd have issues. As I said, I don't think anyone will have issues but listing them was no work for me after the scan so I figured I'd share.
#12
in reply to:
↑ 11
@
8 years ago
Replying to Ipstenu:
@dd32 It was asked in Slack to see who was using it and if they'd have issues. As I said, I don't think anyone will have issues but listing them was no work for me after the scan so I figured I'd share.
Ah cool, no problem either way :)
This ticket was mentioned in Slack in #core by paragonie. View the logs.
8 years ago
#14
follow-up:
↓ 15
@
8 years ago
@dd32 - The risk for breakage should be very minimal, but I would add a recommendation in the 4.9.2 release notes in case it does happen.
If a plugin breaks with this upgrade:
- Replace
random_int()withwp_rand() - Replace
random_bytes()with a function that useswp_rand()to construct a string.
For example:
<?php
/**
* Alternative to random_bytes() that uses wp_rand().
*
* @param int $len
* @return string
* @throws Exception
* @throws TypeError
*/
function wp_random_byte_string($len = 0)
{
if (!is_int($len)) {
throw new TypeError("Length must be an integer.");
}
if ($len < 1) {
throw new Exception("Length must be greater than 0");
}
$chr = '';
for ($i = 0; $i < $len; ++$i) {
// pack('C', $int) is equivalent to chr($int), without cache timing leaks
// See: https://paragonie.com/blog/2017/02/cryptographically-secure-php-development#chr
$chr .= pack('C', wp_rand(0, 255));
}
return $chr;
}
This will allow you gracefully handle degradation. (Feel free to adapt this sample function for core if you want.)
#15
in reply to:
↑ 14
@
8 years ago
Replying to paragoninitiativeenterprises:
@dd32 - The risk for breakage should be very minimal, but I would add a recommendation in the 4.9.2 release notes in case it does happen.
We've not updated the library in 4.9.2, instead I've modified the old version to parse correctly.
It's likely I'll hack the OpenSSL variant back in to be on the safe side as I'm not going to attempt to setup one of these old windows servers.
Diff submission for ticket # 42439