fix(ftp): fix STARTTLS, error visibility, and robustness
Problem: `curl_ftp_url` emitted `ftps://` (implicit TLS) for `oil-ftps://` URLs, causing listing to fail against STARTTLS servers while Python mutations worked — the two paths spoke different TLS modes. curl's `-s` flag silenced all error output, producing "Unknown error" on any curl failure. File creation used `/dev/null` (breaks on Windows, diverges from S3). The Python TLS context didn't honour `--insecure`/`-k` from `extra_curl_args`. Deleting non-empty dirs on servers without MLSD gave a cryptic `500 Unknown command`. Solution: Always emit `ftp://` in `curl_ftp_url`; TLS is enforced solely via `--ssl-reqd`, making STARTTLS consistent between curl and Python. Add `-S` to expose curl errors. Replace `/dev/null` with `curl -T -` + `stdin='null'` (matches `s3fs` pattern). Mirror `--insecure`/`-k` into the Python SSL context. Wrap `mlsd()` in try/except with a clear actionable message. Add `spec/ftp_spec.lua` with 28 unit tests covering URL parsing, list parsing, and curl URL building. Update `doc/oil.txt` to document STARTTLS and MLSD.
This commit is contained in:
parent
0af52a0f6d
commit
27544d73e7
3 changed files with 205 additions and 14 deletions
|
|
@ -1001,8 +1001,9 @@ FTP *oil-adapter-ft
|
||||||
nvim oil-ftp://[username[:password]@]hostname[:port]/[path]/
|
nvim oil-ftp://[username[:password]@]hostname[:port]/[path]/
|
||||||
nvim oil-ftps://[username[:password]@]hostname[:port]/[path]/
|
nvim oil-ftps://[username[:password]@]hostname[:port]/[path]/
|
||||||
<
|
<
|
||||||
The `oil-ftps://` scheme connects with `--ssl-reqd`, requiring a valid TLS
|
The `oil-ftps://` scheme uses explicit TLS (STARTTLS / AUTH TLS, RFC 4217).
|
||||||
certificate. Use `oil-ftp://` for plain FTP.
|
The server must support the `AUTH TLS` command on port 21. Servers using
|
||||||
|
implicit TLS (port 990) are not supported. Use `oil-ftp://` for plain FTP.
|
||||||
|
|
||||||
Authentication ~
|
Authentication ~
|
||||||
|
|
||||||
|
|
@ -1022,7 +1023,9 @@ FTP *oil-adapter-ft
|
||||||
|
|
||||||
Symbolic links cannot be created over FTP. Directory copies are not
|
Symbolic links cannot be created over FTP. Directory copies are not
|
||||||
supported (copy individual files instead). Moving or copying directories
|
supported (copy individual files instead). Moving or copying directories
|
||||||
across different FTP hosts is not supported.
|
across different FTP hosts is not supported. Deleting non-empty directories
|
||||||
|
requires MLSD support (RFC 3659); supported by vsftpd, ProFTPD, FileZilla
|
||||||
|
Server, and most servers from 2007 onwards.
|
||||||
|
|
||||||
Configuration ~
|
Configuration ~
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -78,16 +78,17 @@ end
|
||||||
---@param s string
|
---@param s string
|
||||||
---@return string
|
---@return string
|
||||||
local function url_encode_path(s)
|
local function url_encode_path(s)
|
||||||
return (s:gsub('[^A-Za-z0-9%-._~:/]', function(c)
|
return (
|
||||||
return string.format('%%%02X', c:byte())
|
s:gsub('[^A-Za-z0-9%-._~:/]', function(c)
|
||||||
end))
|
return string.format('%%%02X', c:byte())
|
||||||
|
end)
|
||||||
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
---@param url oil.ftpUrl
|
---@param url oil.ftpUrl
|
||||||
---@return string
|
---@return string
|
||||||
local function curl_ftp_url(url)
|
local function curl_ftp_url(url)
|
||||||
local scheme = url.scheme == 'oil-ftps://' and 'ftps://' or 'ftp://'
|
local pieces = { 'ftp://' }
|
||||||
local pieces = { scheme }
|
|
||||||
if url.user then
|
if url.user then
|
||||||
table.insert(pieces, url.user)
|
table.insert(pieces, url.user)
|
||||||
if url.password then
|
if url.password then
|
||||||
|
|
@ -112,8 +113,14 @@ local function ftpcmd(url, py_lines, cb)
|
||||||
local lines = {}
|
local lines = {}
|
||||||
local use_tls = url.scheme == 'oil-ftps://'
|
local use_tls = url.scheme == 'oil-ftps://'
|
||||||
if use_tls then
|
if use_tls then
|
||||||
|
local insecure = vim.tbl_contains(config.extra_curl_args, '--insecure')
|
||||||
|
or vim.tbl_contains(config.extra_curl_args, '-k')
|
||||||
table.insert(lines, 'import ftplib, ssl')
|
table.insert(lines, 'import ftplib, ssl')
|
||||||
table.insert(lines, 'ctx = ssl.create_default_context()')
|
table.insert(lines, 'ctx = ssl.create_default_context()')
|
||||||
|
if insecure then
|
||||||
|
table.insert(lines, 'ctx.check_hostname = False')
|
||||||
|
table.insert(lines, 'ctx.verify_mode = ssl.CERT_NONE')
|
||||||
|
end
|
||||||
table.insert(lines, 'ftp = ftplib.FTP_TLS(context=ctx)')
|
table.insert(lines, 'ftp = ftplib.FTP_TLS(context=ctx)')
|
||||||
else
|
else
|
||||||
table.insert(lines, 'import ftplib')
|
table.insert(lines, 'import ftplib')
|
||||||
|
|
@ -154,13 +161,18 @@ end
|
||||||
|
|
||||||
---@param url oil.ftpUrl
|
---@param url oil.ftpUrl
|
||||||
---@param extra_args string[]
|
---@param extra_args string[]
|
||||||
---@param cb fun(err: nil|string, output: nil|string[])
|
---@param opts table|fun(err: nil|string, output: nil|string[])
|
||||||
local function curl(url, extra_args, cb)
|
---@param cb? fun(err: nil|string, output: nil|string[])
|
||||||
local cmd = { 'curl', '-s', '--netrc-optional' }
|
local function curl(url, extra_args, opts, cb)
|
||||||
|
if not cb then
|
||||||
|
cb = opts
|
||||||
|
opts = {}
|
||||||
|
end
|
||||||
|
local cmd = { 'curl', '-sS', '--netrc-optional' }
|
||||||
vim.list_extend(cmd, ssl_args(url))
|
vim.list_extend(cmd, ssl_args(url))
|
||||||
vim.list_extend(cmd, config.extra_curl_args)
|
vim.list_extend(cmd, config.extra_curl_args)
|
||||||
vim.list_extend(cmd, extra_args)
|
vim.list_extend(cmd, extra_args)
|
||||||
shell.run(cmd, cb)
|
shell.run(cmd, opts, cb)
|
||||||
end
|
end
|
||||||
|
|
||||||
---@param url oil.ftpUrl
|
---@param url oil.ftpUrl
|
||||||
|
|
@ -498,7 +510,7 @@ M.perform_action = function(action, cb)
|
||||||
elseif action.entry_type == 'link' then
|
elseif action.entry_type == 'link' then
|
||||||
cb('FTP does not support symbolic links')
|
cb('FTP does not support symbolic links')
|
||||||
else
|
else
|
||||||
curl(res, { '-T', '/dev/null', curl_ftp_url(res) }, cb)
|
curl(res, { '-T', '-', curl_ftp_url(res) }, { stdin = 'null' }, cb)
|
||||||
end
|
end
|
||||||
elseif action.type == 'delete' then
|
elseif action.type == 'delete' then
|
||||||
local res = M.parse_url(action.url)
|
local res = M.parse_url(action.url)
|
||||||
|
|
@ -506,7 +518,12 @@ M.perform_action = function(action, cb)
|
||||||
if action.entry_type == 'directory' then
|
if action.entry_type == 'directory' then
|
||||||
ftpcmd(res, {
|
ftpcmd(res, {
|
||||||
'def rmtree(f, p):',
|
'def rmtree(f, p):',
|
||||||
' for name, facts in f.mlsd(p):',
|
' try:',
|
||||||
|
' entries = list(f.mlsd(p))',
|
||||||
|
' except ftplib.error_perm as e:',
|
||||||
|
' if "500" in str(e) or "502" in str(e): import sys; sys.exit("Server does not support MLSD; cannot recursively delete non-empty directories")',
|
||||||
|
' raise',
|
||||||
|
' for name, facts in entries:',
|
||||||
' if name in (".", ".."): continue',
|
' if name in (".", ".."): continue',
|
||||||
' child = p.rstrip("/") + "/" + name',
|
' child = p.rstrip("/") + "/" + name',
|
||||||
' if facts["type"] == "dir": rmtree(f, child)',
|
' if facts["type"] == "dir": rmtree(f, child)',
|
||||||
|
|
@ -654,4 +671,9 @@ M.write_file = function(bufnr)
|
||||||
end)
|
end)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
M._parse_unix_list_line = parse_unix_list_line
|
||||||
|
M._parse_iis_list_line = parse_iis_list_line
|
||||||
|
M._url_encode_path = url_encode_path
|
||||||
|
M._curl_ftp_url = curl_ftp_url
|
||||||
|
|
||||||
return M
|
return M
|
||||||
|
|
|
||||||
166
spec/ftp_spec.lua
Normal file
166
spec/ftp_spec.lua
Normal file
|
|
@ -0,0 +1,166 @@
|
||||||
|
local ftp = require('oil.adapters.ftp')
|
||||||
|
|
||||||
|
describe('ftp adapter', function()
|
||||||
|
describe('parse_url', function()
|
||||||
|
it('parses minimal url', function()
|
||||||
|
local res = ftp.parse_url('oil-ftp://host/')
|
||||||
|
assert.equals('oil-ftp://', res.scheme)
|
||||||
|
assert.equals('host', res.host)
|
||||||
|
assert.equals('', res.path)
|
||||||
|
assert.equals(nil, res.user)
|
||||||
|
assert.equals(nil, res.password)
|
||||||
|
assert.equals(nil, res.port)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('parses url with user, password, port, and path', function()
|
||||||
|
local res = ftp.parse_url('oil-ftp://user:pass@host:2121/some/path/')
|
||||||
|
assert.equals('oil-ftp://', res.scheme)
|
||||||
|
assert.equals('host', res.host)
|
||||||
|
assert.equals('user', res.user)
|
||||||
|
assert.equals('pass', res.password)
|
||||||
|
assert.equals(2121, res.port)
|
||||||
|
assert.equals('some/path/', res.path)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('parses url with user only', function()
|
||||||
|
local res = ftp.parse_url('oil-ftp://user@host/path/')
|
||||||
|
assert.equals('user', res.user)
|
||||||
|
assert.equals(nil, res.password)
|
||||||
|
assert.equals(nil, res.port)
|
||||||
|
assert.equals('path/', res.path)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('parses oil-ftps:// scheme', function()
|
||||||
|
local res = ftp.parse_url('oil-ftps://host/path/')
|
||||||
|
assert.equals('oil-ftps://', res.scheme)
|
||||||
|
assert.equals('host', res.host)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('parses port without credentials', function()
|
||||||
|
local res = ftp.parse_url('oil-ftp://host:990/')
|
||||||
|
assert.equals('host', res.host)
|
||||||
|
assert.equals(990, res.port)
|
||||||
|
assert.equals(nil, res.user)
|
||||||
|
end)
|
||||||
|
end)
|
||||||
|
|
||||||
|
describe('get_parent', function()
|
||||||
|
it('goes up one directory', function()
|
||||||
|
assert.equals('oil-ftp://host/', ftp.get_parent('oil-ftp://host/subdir/'))
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('goes up nested directories', function()
|
||||||
|
assert.equals(
|
||||||
|
'oil-ftp://user:pass@host:2121/a/b/',
|
||||||
|
ftp.get_parent('oil-ftp://user:pass@host:2121/a/b/c/')
|
||||||
|
)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('stays at root', function()
|
||||||
|
assert.equals('oil-ftp://host/', ftp.get_parent('oil-ftp://host/'))
|
||||||
|
end)
|
||||||
|
end)
|
||||||
|
|
||||||
|
describe('unix list line parsing', function()
|
||||||
|
it('parses a regular file', function()
|
||||||
|
local name, entry_type, meta =
|
||||||
|
ftp._parse_unix_list_line('-rw-r--r-- 1 user group 42 Jan 15 2024 hello.txt')
|
||||||
|
assert.equals('hello.txt', name)
|
||||||
|
assert.equals('file', entry_type)
|
||||||
|
assert.equals(42, meta.size)
|
||||||
|
assert.equals('number', type(meta.mtime))
|
||||||
|
assert.equals(420, meta.mode)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('parses a directory', function()
|
||||||
|
local name, entry_type, meta =
|
||||||
|
ftp._parse_unix_list_line('drwxr-xr-x 2 user group 4096 Mar 18 10:30 subdir')
|
||||||
|
assert.equals('subdir', name)
|
||||||
|
assert.equals('directory', entry_type)
|
||||||
|
assert.equals(4096, meta.size)
|
||||||
|
assert.equals('number', type(meta.mtime))
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('parses a symlink', function()
|
||||||
|
local name, entry_type, meta =
|
||||||
|
ftp._parse_unix_list_line('lrwxrwxrwx 1 user group 8 Mar 18 10:30 link -> target')
|
||||||
|
assert.equals('link', name)
|
||||||
|
assert.equals('link', entry_type)
|
||||||
|
assert.equals('target', meta.link)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('parses a filename with spaces', function()
|
||||||
|
local name, entry_type, _ =
|
||||||
|
ftp._parse_unix_list_line('-rw-r--r-- 1 user group 100 Mar 15 09:00 my file.txt')
|
||||||
|
assert.equals('my file.txt', name)
|
||||||
|
assert.equals('file', entry_type)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('returns nil for unrecognised lines', function()
|
||||||
|
local name = ftp._parse_unix_list_line('total 42')
|
||||||
|
assert.equals(nil, name)
|
||||||
|
end)
|
||||||
|
end)
|
||||||
|
|
||||||
|
describe('iis list line parsing', function()
|
||||||
|
it('parses a directory', function()
|
||||||
|
local name, entry_type, _ =
|
||||||
|
ftp._parse_iis_list_line('01-14-24 09:27AM <DIR> dirname')
|
||||||
|
assert.equals('dirname', name)
|
||||||
|
assert.equals('directory', entry_type)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('parses a file', function()
|
||||||
|
local name, entry_type, meta =
|
||||||
|
ftp._parse_iis_list_line('01-14-24 09:27AM 12345 file.txt')
|
||||||
|
assert.equals('file.txt', name)
|
||||||
|
assert.equals('file', entry_type)
|
||||||
|
assert.equals(12345, meta.size)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('returns nil for unrecognised lines', function()
|
||||||
|
local name = ftp._parse_iis_list_line('drwxr-xr-x 2 user group 4096 Mar 18 10:30 subdir')
|
||||||
|
assert.equals(nil, name)
|
||||||
|
end)
|
||||||
|
end)
|
||||||
|
|
||||||
|
describe('curl_ftp_url', function()
|
||||||
|
it('always uses ftp:// scheme for oil-ftp://', function()
|
||||||
|
local url = ftp.parse_url('oil-ftp://host/path/')
|
||||||
|
assert(vim.startswith(ftp._curl_ftp_url(url), 'ftp://'))
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('always uses ftp:// scheme for oil-ftps://', function()
|
||||||
|
local url = ftp.parse_url('oil-ftps://host/path/')
|
||||||
|
assert(vim.startswith(ftp._curl_ftp_url(url), 'ftp://'))
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('encodes spaces in path', function()
|
||||||
|
local url = ftp.parse_url('oil-ftp://host/my path/')
|
||||||
|
assert.equals('ftp://host/my%20path/', ftp._curl_ftp_url(url))
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('includes credentials and port', function()
|
||||||
|
local url = ftp.parse_url('oil-ftp://user:pass@host:2121/dir/')
|
||||||
|
assert.equals('ftp://user:pass@host:2121/dir/', ftp._curl_ftp_url(url))
|
||||||
|
end)
|
||||||
|
end)
|
||||||
|
|
||||||
|
describe('url_encode_path', function()
|
||||||
|
it('leaves safe characters unchanged', function()
|
||||||
|
assert.equals('hello.txt', ftp._url_encode_path('hello.txt'))
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('encodes spaces', function()
|
||||||
|
assert.equals('a%20file.txt', ftp._url_encode_path('a file.txt'))
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('preserves path separators', function()
|
||||||
|
assert.equals('dir/subdir/file.txt', ftp._url_encode_path('dir/subdir/file.txt'))
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('encodes special characters', function()
|
||||||
|
assert.equals('hello%23world', ftp._url_encode_path('hello#world'))
|
||||||
|
end)
|
||||||
|
end)
|
||||||
|
end)
|
||||||
Loading…
Add table
Add a link
Reference in a new issue