WordPress.org

1. Dion Hulse (@dd32)
   WordPress Dev
   Posted 9 months ago #

   Hey,
   Just to let you know, There's still a few issues with paths.

   1. NEVER hard code wp-content in the plugin, that's just asking for trouble for those of us who use custom WP_CONTENT_DIR's.. probably best to use dirname(__FILE__) instead maybe?
   2. The Include path is still set incorrectly in class-wp-filesystem-ssh2.php although it's set right in sftp.php so it's not really a problem
   3. You seem to be attempting to force ssl usage through the filesystem_method filter via phpseclib_filesystem_method(), however this doesn't actually work, as it's hooked via add_action() rather than add_filter() - I'd suggest removing that action/function myself, and let the radio selections take over
   4. When running with WP_DEBUG mode, phpseclib throws a PHP notice for an undefined constant on every file operation: Notice: Use of undefined constant NET_SFTP_APPEND - assumed 'NET_SFTP_APPEND' in ssh-sftp-updater-support/phpseclib/Net/SFTP.php on line 1282
   5. I notice in the chmod() method, you're not setting it if it's called with a false mode, Are you aware of why it's being called with a false mode? Core sets it for every request due to the number of server configurations where not setting it was causing security implications, For SSH, it's probably fine not to set it for every request, but it seems strange that it should ever get called with a false mode.

   I think that's everything.. I've been meaning to ask the above questions as I've been testing it out, Just haven't gotten around to it until now!
   If you've got any questions on any of the upgrade stuff, give me a yell? wordpress at dd32.id.au

2. TerraFrost
   Member
   Posted 8 months ago #

   1. Should be fixed in the latest SVN.
   2. Should be fixed in the latest SVN.
   3. I tried this:
      add_filter('fs_ftp_connection_types', 'phpseclib_fs_ftp_connection_types'); // since 2.9

      function phpseclib_fs_ftp_connection_types($types) {
      $types['ssh'] = __('SSH2');
      return $types;
      }

      But that doesn't seem to do anything. And looking at the code I don't see what it'd do anyway.

      It looks like 'filesystem_method' is called in get_filesystem_method() which, in turn, is called in WP_Filesystem to determine the class that should be loaded.

3. TerraFrost
   Member
   Posted 8 months ago #

   4. Should be fixed in the latest SVN.
   5. From wp-admin/includes/file.php:

   if ( ! $wp_filesystem->copy($from . $filename, $to . $filename, true, FS_CHMOD_FILE) ) {

   FS_CHMOD_FILE isn't defined by default.

4. Dion Hulse (@dd32)
   WordPress Dev
   Posted 8 months ago #

   > 3. I tried this:

   Sorry, It seems I had the ssh PHP extension loaded which was making me think certain parts weren't needed.. When that's not loaded, you do indeed need a few other other hooks.

   > 5. FS_CHMOD_FILE isn't defined by default.

   It's defined late by the WP_Filesystem() function - It's defined after the connect() method has completed, See: http://core.trac.wordpress.org/browser/trunk/wp-admin/includes/file.php#L835 - Thats so that the connect method can define it if need be, classes might want to do a umask() of the server or something..

   2 other pointers I picked up:
   * No need to set the include path / includes in sftp.php, save memory and time for every other request, just include it when the class is loaded
   * Part of my confusion was your usage of add_action, when you were actually adding filters, might as well be explicit in the function you're calling, even if it's an alias to itself. Also added some comments for why each was needed, that was just for my own understanding of each one as I debugged it.
   Both of the above are shown in this patch file: http://dd32.id.au/wordpress/wp-content/plugins/ssh-sftp-updater-support/out.diff

   Other than that, it appears to be working great for me and a few others on my VPS :)

5. TerraFrost
   Member
   Posted 8 months ago #

   The FS_CHMOD_FILE issue has been resolved with the following:

   http://plugins.trac.wordpress.org/changeset/439809

   Thanks for your feedback!!

Reply

You must log in to post.