WordPress.org

Make WordPress Core

Opened 23 months ago

Last modified 6 days ago

#35517 new defect (bug)

Work around PHP7 php-ssh2 breakage

Reported by: dougal Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Filesystem API Keywords: has-patch
Focuses: administration Cc:

Description

There is an updated php-ssh2 package available for PHP7, but it currently breaks the WordPress updater functionality for class-wp-filesystem-ssh2.php. The root cause seems to be that it has not correctly implemented the PHP stream wrappers for the stat() call, and any dependent functions such as is_file(), is_dir(), file_exists(), etc.

However, the ssh2_sftp_stat() function does work, and we can deduce the other information from it.

I've filed a bug against the php-ssh2 extension (https://bugs.php.net/bug.php?id=71376), but I wondered if using ssh2_sftp_stat() might be better, in general, than depending on the PHP stream wrapper functionality.

Attachments (1)

class-wp-filesystem-ssh2.php.patch (2.3 KB) - added by dougal 23 months ago.
Use ssh2_sftp_stat() instead of relying on PHP stream wrappers and allow_url_fopen

Download all attachments as: .zip

Change History (27)

@dougal
23 months ago

Use ssh2_sftp_stat() instead of relying on PHP stream wrappers and allow_url_fopen

#1 @dougal
23 months ago

  • Keywords has-patch added

#3 follow-up: @dd32
23 months ago

I'm rather torn over this. On one hand I want to fix the bug, however, the bug is quite clearly in the extension as it didn't get ported to PHP7 correctly.

If usage of PHP7 + SSH was higher, I'd be more inclined to fix this, however right now this feels like a wontfix assuming the extension gets updated to work properly.

#4 in reply to: ↑ 3 @dougal
23 months ago

Replying to dd32:

I'm rather torn over this. On one hand I want to fix the bug, however, the bug is quite clearly in the extension as it didn't get ported to PHP7 correctly.

If usage of PHP7 + SSH was higher, I'd be more inclined to fix this, however right now this feels like a wontfix assuming the extension gets updated to work properly.

I can understand that. "Not my circus, not my monkeys". Theoretically, it will be a self-correcting problem. And I waffled over whether to file this as a bug or an enhancement in the first place. But just as food for thought...

Do we have any numbers on how many servers have allow_url_fopen turned off? By using the native libssh2 functions instead of relying on the stream wrappers, it might allow a few more systems to function (though I'd have to actually turn that setting off and do real testing before I fully commit to that idea). I don't know if that would affect enough systems to justify the change, though. Especially since it's been stated that overall usage of the ssh2 filesystem isn't terribly high in the first place?

In any case, my patch is working for my own system for now.

#5 follow-up: @dd32
23 months ago

Do we have any numbers on how many servers have allow_url_fopen turned off?

WordPress.org doesn't collect stats on information such as that, and we could switch most of the class to using the ssh2_sftp_* class of functions instead (which wouldn't be restricted by allow_url_fopen) however I'm not sure the intersection between the two is enough to warrant switching.

I will say, eliminating the usage of stream wrappers in preference for the functions seems like a better reason for change than working around the PHP7 breakage.

#6 in reply to: ↑ 5 ; follow-up: @dougal
23 months ago

Replying to dd32:

I will say, eliminating the usage of stream wrappers in preference for the functions seems like a better reason for change than working around the PHP7 breakage.

Would it be worth it for me to re-cast this ticket as a Feature Request for eliminating stream wrapper usage in the class-wp-filesystem-ssh2 code, and see if I need to update my patch for any other methods?

Or should I just install the SSH SFTP Updater Support plugin and forget about it?

#7 in reply to: ↑ 6 @aberbenni
22 months ago

Or should I just install the SSH SFTP Updater Support plugin and forget about it?

SSH SFTP Updater Support does not pass tests with Wp FS Tester, look here ticket:35946#comment:2

#8 @dd32
22 months ago

#35946 was marked as a duplicate.

#9 @aberbenni
21 months ago

Even with the patch I'm having this:

 PHP Warning:  file_get_contents(ssh2.sftp:///var/www/www.website.com/website/wp-content/uploads/js_composer/custom.css): failed to open stream: operation failed in /var/www/www.website.com/website/wp-admin/includes/class-wp-filesystem-ssh2.php on line 223
 PHP Stack trace:
 PHP   1. {main}() /var/www/www.website.com/website/index.php:0
 PHP   2. require() /var/www/www.website.com/website/index.php:17
 PHP   3. require_once() /var/www/www.website.com/website/wp-blog-header.php:16
 PHP   4. do_action() /var/www/www.website.com/website/wp-includes/template-loader.php:12
 PHP   5. Vc_Base->frontCss() /var/www/www.website.com/website/wp-includes/plugin.php:525
 PHP   6. vc_file_get_contents() /var/www/www.website.com/website/wp-content/plugins/js_composer/include/classes/core/class-vc-base.php:549
 PHP   7. WP_Filesystem_SSH2->get_contents() /var/www/www.website.com/website/wp-content/plugins/js_composer/include/helpers/helpers_factory.php:374
 PHP   8. file_get_contents() /var/www/www.website.com/website/wp-admin/includes/class-wp-filesystem-ssh2.php:223

#10 @dd32
21 months ago

@aberbenni Thanks for reporting back on it!

Unfortunately that error is purely within the land of the SSH2 extension and it's communication with your SSH server, there's literally nothing we can really do (other than silence the error, and possibly retry every time it happens) in WordPress to work around that.

#11 @mikeschroder
21 months ago

  • Version trunk deleted

#12 @dd32
15 months ago

#37942 was marked as a duplicate.

#13 @dd32
15 months ago

#37942 was marked as a duplicate.

As I've marked another ticket as a duplicate of this, I'd just like to express my personal thoughts here.

I personally don't really want to touch the internals of this transport, due to it's limited use, and how rare it is that it's supported.
Testing this change on older versions of the extension, interactions with ssh servers, and my experience of changing internals of transports has told me that if you fix it for one configuration, it'll break ten others.
If the PHP Extension-supplied bindings are faulty, the extension should be fixed instead (which it is, hopefully).

#14 @jobst
10 months ago

(including @dd32)
I really do not understand that the php-ssh2 extension is not used more often!

When you use the direct update FS_METHOD you have to open the entire file tree to the user of the web-server as writeable, otherwise the FS_METHOD=direct does not work.

The FS_METHOD=ssh2 has made me sleep so much better as I can have two users within the entire tree of a Wordpress installation, where the OWNER is the only one being able to write the entire tree and the web-server user can only write a very limited range (e.g. ./wp-content/[upload|cache|upgrade|wf*] plus some css directories).

The php-ssh2 extension allows me to specify the OWNER of the tree as the updater and thus I have no problems updating. It's so simple to stop a malicious script to overwrite index.php using the php-ssh2 extension.

#15 follow-ups: @stianstr23
9 months ago

Hi,

Compiling ssh2 from latest master at https://github.com/php/pecl-networking-ssh2 fixes this issue.

#16 in reply to: ↑ 15 @jobst
9 months ago

Replying to stianstr23:

Compiling ssh2 from latest master at https://github.com/php/pecl-networking-ssh2 fixes this issue.

Not quite, I tried this and it did not work. While it fixes some of the issues the problem with the different ways of handling resources in PHP (just the number) and SSH2 (#resource 123) still has problems. I simply replaced the calls to standard PHP functions (e.g. exist) with calls from the ssh2 lib - that works.

See #39746.

#17 @andythiel
8 months ago

#40541 was marked as a duplicate.

#18 @andythiel
8 months ago

I understand the reasoning that it is hard to fix and that the library needs to be fixed ...
but usage is an argument that surprises me.

I fear many people (those new to Wordpress who use Ubuntu 16.04 and are not hardcore Admins) fix this issue by granting permissions to www-data and setting FS_METHOD to "direct". At least that is the impression that I got from StackExchange like sites.

For me the fixes in the diff above did not solve the issue.
Deleting and Creating folders does not seem to work. Not sure where to look for exact error logging. It might be the same thing aberbenni mentioned or I messed up with copy&paste which the information below suggests.

I too see no other way than giving up on ssh2 (but have a very bad feeling about it).
Hope this gets resolved soon. +1 regarding priority from my side, for whatever that's worth.

Some probably irrelevant information from the Wp FS Tester with the fixes above (probably should not contain any php code ... so yeah maybe I messed up copying the fixes but I double checked):

Files in folder	
		
Plugin location:	/var/www/firsttechfirst.com/wp-content/plugins/wp-filesystem-tester/wp-filesystem-tester.php (Local)
/var/www/firsttechfirst.com/wp-content/plugins/wp-filesystem-tester/wp-filesystem-tester.php (FTP)
Plugin Locations	
hello.php	/var/www/firsttechfirst.com/wp-content/plugins/	Delete file: /var/www/firsttechfirst.com/wp-content/plugins/hello.php
akismet/akismet.php	/var/www/firsttechfirst.com/wp-content/plugins/akismet/	Delete entire folder: /var/www/firsttechfirst.com/wp-content/plugins/akismet/
File IO Errors	Error: Could not create file /var/www/firsttechfirst.com/wp-content/plugins/super-long-name-not-to-conflict.php on server
Downloading a zip	Downloading http://downloads.wordpress.org/plugin/akismet.zip... Suceeded
Extracting Zip	Suceeded
Zip Contents	
akismet/	0	ok	binary
akismet/akismet.php	2409	ok	<?php
/**
 * @package Akismet
 */
/*
Plugin Name: ...
akismet/class.akismet-admin.php	43023	ok	<?php

class Akismet_Admin {
	const NONCE = 'akism...
akismet/class.akismet-cli.php	2712	ok	<?php

WP_CLI::add_command( 'akismet', 'Akismet_CL...
akismet/class.akismet.php	48848	ok	<?php

class Akismet {
	const API_HOST = 'rest.aki...
akismet/class.akismet-widget.php	2825	ok	<?php
/**
 * @package Akismet
 */
class Akismet_Wi...
akismet/.htaccess	629	ok	binary
akismet/_inc/	0	ok	binary
akismet/_inc/akismet.css	11547	ok	binary
akismet/_inc/akismet.js	6921	ok	binary
akismet/_inc/form.js	700	ok	binary
akismet/_inc/img/	0	ok	binary
akismet/_inc/img/logo-full-2x.png	7570	ok	binary
akismet/index.php	26	ok	<?php
# Silence is golden....
akismet/LICENSE.txt	18092	ok	                    GNU GENERAL PUBLIC LICENSE
   ...
akismet/readme.txt	15033	ok	=== Akismet ===
Contributors: matt, ryan, andy, md...
akismet/views/	0	ok	binary
akismet/views/config.php	10608	ok	<div id="akismet-plugin-container">
	<div class="a...
akismet/views/get.php	608	ok	<form name="akismet_activate" action="https://akis...
akismet/views/notice.php	9148	ok	<?php if ( $type == 'plugin' ) :?>
<div class="upd...
akismet/views/start.php	6757	ok	<div id="akismet-plugin-container">
	<div class="a...
akismet/views/stats.php	744	ok	<div id="akismet-plugin-container">
	<div class="a...
akismet/wrapper.php	6437	ok	<?php

global $wpcom_api_key, $akismet_api_host, $...

#19 @ocean90
6 months ago

#41030 was marked as a duplicate.

#20 @supaiku
4 months ago

For some reason the patch didn't resolve it for me, but I found this sketch work around:
https://wordpress.org/support/topic/php-7-compatibility-71/#post-9363511

The SSH plugin, and then someone suggested one. Is it really a safe work-around? :P

#21 in reply to: ↑ 15 @ondrique
3 months ago

Replying to stianstr23:

Hi,

Compiling ssh2 from latest master at https://github.com/php/pecl-networking-ssh2 fixes this issue.

Yes,

I can confirm that updating to the versions below via PECL worked for me too:

SSH2
extension version 1.1.2
libssh2 version 1.5.0

#22 @mangoo
3 months ago

#41831 was marked as a duplicate.

#23 @ralphm
8 weeks ago

I'm not entirely sure if this is the same problem, but I got upgrades using ssh2 to work with this patch:

--- a/wp-admin/includes/class-wp-filesystem-ssh2.php
+++ b/wp-admin/includes/class-wp-filesystem-ssh2.php
@@ -185,7 +185,7 @@
 		if ( '/' === $path ) {
 			$path = '/./';
 		}
-		return 'ssh2.sftp://' . $this->sftp_link . '/' . ltrim( $path, '/' );
+		return 'ssh2.sftp://' . intval($this->sftp_link) . '/' . ltrim( $path, '/' );
 	}
 
 	/**

#24 @kareltje63
6 weeks ago

Solved by @ralphm's patch!
Had WordPress-does-not-update-through-ssh2 problem on ubuntu 14.04, upgraded PHP ssh2 to no avail, then upgraded to Ubuntu 16.04, PHP 7, a slightly better error message (instead of an empty page a 'cannot locate wp-content' message), but still not able to upgrade.

Then I applied the intval() patch for file wp-admin/includes/class-wp-filesystem-ssh2.php which solved the problem for me on both the original ubuntu 14.04 WordPress install and the 16.04 install.

Do not forget to apply the patch again after upgrading, until the patch makes it into a new WordPress release

#25 @2bitcoders
3 weeks ago

Solved by @ralphm's patch on Ubuntu 14.04.5 LTS. After an apt-get update, the issue started. Did a manual update of WP and the issue remained.

Used his update patch and the update-core.php went from 404's to working again.

#26 @SoenHien
6 days ago

Solved by @ralphm's patch!

Problem started after apt-get update & upgrade to Ubuntu 14.04. Apache2 segment fault when installing/updating wordpress plugin/theme.

Confirmed: intval() patch solved it. THANKS.

Note: See TracTickets for help on using tickets.