Differential D21290

kio_ftp: improve error handling when copying to FTP fails
ClosedPublic
Actions

Authored by dfaure on May 19 2019, 1:01 PM.

Details

Reviewers
bruns
apol
kossebau
Commits
R241:2e37ace712ae: kio_ftp: improve error handling when copying to FTP fails
Summary

Before:

Could not write to file /index.html.part
... and WARNING: finished() called after error()! Please fix the "kio_ftp" KIO slave

After:

Could not write to file /index.html.part
The server said: "550 Can't open that file: Disk quota exceeded."
... and no more warning.
Test Plan

Copying a huge file onto an ftp folder in dolphin.
The server is running Pure-FTPd.

Diff Detail

Repository
R241 KIO
Branch
2019_freebsd_fixed
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 12178
Build 12196: arc lint + arc unit
dfaure requested review of this revision.May 19 2019, 1:01 PM
dfaure created this revision.
pino added a subscriber: pino.May 19 2019, 1:05 PM
pino added inline comments.
src/ioslaves/ftp/ftp.cpp
1085

please use a placeholder for the server response in the i18n string

dfaure updated this revision to Diff 58301.May 19 2019, 1:23 PM

Use %1 placeholder, add double quotes

bruns added inline comments.May 25 2019, 4:15 PM
src/ioslaves/ftp/ftp.cpp
1085

From RFC 959 4.2. FTP REPLIES:

Note that the text
associated with each reply is recommended, rather than
mandatory, and may even change according to the command with
which it is associated.  The reply codes, on the other hand,
must strictly follow the specifications in the last section;
that is, Server implementations should not invent new codes for
situations that are only slightly different from the ones
described here, but rather should adapt codes already defined.

So if the response code is chopped away, there may be no useful message left.
Maybe i18n("\nServer response: [%1] \"%2\"", code, message)?

bruns added a comment.May 25 2019, 4:17 PM

Can you also add to the summary which FTP server implementation you tested against?

dfaure marked an inline comment as done.May 26 2019, 3:45 PM
dfaure added inline comments.
src/ioslaves/ftp/ftp.cpp
1085

OK, simplest then is to keep the full response. I was just trying to make it more user friendly ;-)

dfaure updated this revision to Diff 58687.May 26 2019, 3:46 PM
dfaure edited the summary of this revision. (Show Details)
dfaure edited the test plan for this revision. (Show Details)

keep error code; mention pure-ftpd in the commit log

Restricted Application added a project: Frameworks. · View Herald TranscriptMay 26 2019, 3:46 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
bruns added inline comments.May 26 2019, 8:20 PM
src/ioslaves/ftp/ftp.cpp
2356–2357

ftpCopyPut calls iCopyFIle = QT_OPEN before calling ftpPut -> ftpSendCmd, so this leaks.

dfaure updated this revision to Diff 58694.May 26 2019, 8:36 PM

Don't leak the file descriptor on error. Thanks for catching!

bruns accepted this revision.Jun 2 2019, 11:27 AM
This revision is now accepted and ready to land.Jun 2 2019, 11:27 AM
dfaure closed this revision.Jun 2 2019, 7:27 PM

Revision Contents
Changeset List

PathPackages
Msrc/ioslaves/ftp/ftp.cpp (20 lines)

Diff 58694

View Options

src/ioslaves/ftp/ftp.cpp

Loading...