invenia
Escape special chars in username/password to avoid building incorrect URIs
When either the username or password contains special chars (such as @ or :), these have to be escaped in order for the FTP URI to be correctly parseable (both by the remote FTP server, and downstream FTPClient.jl functions, such as safe_uri)
This PR does that when the FTP object is built with individual keyword arguments. A similar problem occurs when building the FTP object with an uri (#111) but it could be argued that in this case it is the user's responsibility to provide a parseable URI in which problematic characters have already been escaped.
Thanks for your reply!
Entirely agree about tests. I actually was about to write some new tests for this, but I didn't manage to make existing tests work: after installing the required python libs (openssl and pyftpdlib), I get a few errors:
Test Summary: | Pass Error Total Time
all_tests | 328 9 337 2m32.0s
All Tests | 321 9 330 2m31.9s
connection error | 1 1 0.2s
object | 2 2 0.2s
broadcastable | 1 1 0.0s
connection with url | 1 1 0.0s
readdir | 4 4 0.3s
download | 3 3 0.0s
upload | 33 33 0.2s
mkdir | 14 14 0.0s
cd | 12 12 0.0s
rmdir | 9 9 0.0s
pwd | 3 3 0.0s
mv | 8 8 0.0s
rm | 9 9 0.0s
upload | 8 1 9 1.1s
write | 1 1 2 0.1s
verbose | 75 1 76 0.6s
FTP | 1 1 0.1s
readdir | 1 1 0.1s
download | 4 4 0.0s
upload | 8 8 0.1s
mkdir | 7 7 0.0s
cd | 4 4 0.0s
rmdir | 5 5 0.0s
pwd | 4 4 0.0s
mv | 10 10 0.0s
rm | 8 8 0.0s
upload | 17 1 18 0.2s
uploading a file with only the local file name | 8 8 0.0s
uploading a file with remote local file name | 1 1 2 0.1s
upload with retry single file | 8 8 0.0s
verbose multiple commands | 5 5 0.1s
ftp open do end | 1 1 0.0s
active_mode | 122 122 0.5s
binary_ascii | 13 5 18 0.3s
ftp errors | 2 2 0.0s
ssl | 0 2m27.7s
ERROR: LoadError: Some tests did not pass: 328 passed, 0 failed, 9 errored, 0 broken.
in expression starting at /tmp/FTPClient.jl/test/runtests.jl:27
The 5 binary_ascii failures are actually @broken_test that now work. I haven't looked much into the other tests failing, but it might be interesting to note that one of the failures is actually due to me interrupting the process after it (seemingly) hanged for a few minutes.
Is that expected? Or do I have something broken in my test environment?
Anyways, all this is to say that I'd be willing to add tests to this, but I'd probably need a bit of help first to make the existing suite pass.
I just pushed two new commits aiming at repairing the test suite.
Some test failures were actually caused by an issue very similar to the one that prompted this PR: special chars in remote filenames need to be escaped too when building URIs. 4 tests in the suite triggered an issue because of a whitespace in the file names used to test the upload function. This should also fix issues with the download function (#124), but that is not tested (yet).
The 5 tests related to binary/ascii modes seem to work again. Most likely the incompatibilities between libcurl and pyftpdlib mentioned in #113 have been fixed upstream. I therefore re-activated these tests.
The test suite is now in much better shape; the only remaining issue I encounter is related to the two SSL tests: they seem to hang forever on my system. I have to admit I have no idea how to fix these ones.
In any case, we're now missing at least 3 tests related to escaping special chars in:
- username
- password
- remote filename when downloading (upload is already covered)
I added the missing tests. @omus please let me know if that seems to be enough in your opinion.