By default, SFTP is not listed when setting up an authorize.php connection to upload a new module, unless it's known to be available. FTP, however, is.

Except! A lot of servers don't have FTP installed because it's inherently insecure. For instance, my dev server. :-) Even though it's connecting to localhost, FTP isn't available.

We can't do anything about that, of course, but currently even if that's the case FTP is still listed as an available option. That's misleading. The error message, "Cannot connect to FTP Server, check settings", is also misleading, as I cannot connect to it because there isn't one. That's different than having the wrong credentials.

Comments

dww’s picture

dww
we/he/they
commented
Component: update.module » base system
Status: Active » Closed (duplicate)
Issue tags: +Update manager

You might call this a bug in our FileTransfer class (that's where the exception is being thrown). It's not a bug in update.module per se.

However, I think that we're sort of screwed here, since all the client (i.e. Drupal core) can know (at least at the time it's deciding what file transfer mechanisms it can present to the user) is if it knows how to use FTP to connect (e.g. PHP was compiled with FTP support, etc), not if the FTP server running on localhost is actually alive and responding. In theory, we could massively complicate (aka "bloat") the code to try connecting via every possible method, check error codes and exceptions, and try to figure out if there's an FTP server running at all. But that seems like it would be extremely fragile code, likely to cause problems and false positives in various configurations, and probably wouldn't make it into core at all due to size and complexity.

So, this strikes me more as a UI bug in the error reporting and propagation. It'd be nice to clarify this exception string to indicate that one likely reason you can't connect to the FTP server is that it might not exist at all, it might not be listening to you, etc. Therefore, this is duplicate with #605292: propagate failure during batches in update manager (although it's a useful case to try to deal with explicitly, since it's one that's going to be common). Lots of possible exceptions thrown in various error cases are very poorly or tersely worded. None of this UI text got a proper UI/UX review as all this code was going into core in the first place. I was doing a huge amount of refactoring, bug fixing, and cleanup in very short time on a tight deadline, and there was only so much I could do. Since then, in spite of numerous attempts, I've gotten practically no outside help on any of this code, so important UX bugs like #605292 have gone unloved...

Feel free to revert the status, but I'd rather just see effort for fixing this go into #605292.

Thanks,
-Derek

skessler’s picture

skessler
He/Him
Location Denver
commented
Version: 7.0-alpha2 » 7.0-alpha6

I do not understand how this is a duplicate. When I use my test server it does not have PHP and it just not show me a secure connection which would work. If I understand this issue it is different than other issues. This has serious usability ramifications.