fix(ftp): use ftp.rename() for RNFR/RNTO and raw Python lines in ftpcmd

Problem: `ftpcmd` wrapped every command in `ftp.voidcmd()`, which
expects a final 2xx response. `RNFR` returns 350 (intermediate),
so `voidcmd` raised an exception before `RNTO` was ever sent,
causing every rename to fail with '350 Ready for destination name'.

Solution: change `ftpcmd` to accept raw Python lines instead of FTP
command strings, then use `ftp.rename(src, dst)` for the rename case.
`ftplib.rename` handles the 350 intermediate response correctly
internally. All other callers now wrap their FTP commands in
`ftp.voidcmd()` explicitly.
This commit is contained in:
Barrett Ruth 2026-03-17 22:14:31 -04:00
parent ba27fe176b
commit c90c5fcfaa
Signed by: barrett
GPG key ID: A6C96C9349D2FC81

View file

@ -98,9 +98,9 @@ local function curl_ftp_url(url)
end end
---@param url oil.ftpUrl ---@param url oil.ftpUrl
---@param commands string[] ---@param py_lines string[]
---@param cb fun(err: nil|string) ---@param cb fun(err: nil|string)
local function ftpcmd(url, commands, cb) 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
@ -121,8 +121,8 @@ local function ftpcmd(url, commands, cb)
if use_tls then if use_tls then
table.insert(lines, 'ftp.prot_p()') table.insert(lines, 'ftp.prot_p()')
end end
for _, cmd in ipairs(commands) do for _, line in ipairs(py_lines) do
table.insert(lines, string.format('ftp.voidcmd(%q)', cmd)) table.insert(lines, line)
end end
table.insert(lines, 'ftp.quit()') table.insert(lines, 'ftp.quit()')
local script = table.concat(lines, '\n') local script = table.concat(lines, '\n')
@ -360,7 +360,11 @@ ftp_columns.permissions = {
local res = M.parse_url(action.url) local res = M.parse_url(action.url)
local octal = permissions.mode_to_octal_str(action.value) local octal = permissions.mode_to_octal_str(action.value)
local ftp_path = ftp_abs_path(res) local ftp_path = ftp_abs_path(res)
ftpcmd(res, { string.format('SITE CHMOD %s %s', octal, ftp_path) }, callback) ftpcmd(
res,
{ string.format('ftp.voidcmd(%q)', 'SITE CHMOD ' .. octal .. ' ' .. ftp_path) },
callback
)
end, end,
} }
@ -482,7 +486,7 @@ M.perform_action = function(action, cb)
local res = M.parse_url(action.url) local res = M.parse_url(action.url)
local ftp_path = ftp_abs_path(res) local ftp_path = ftp_abs_path(res)
if action.entry_type == 'directory' then if action.entry_type == 'directory' then
ftpcmd(res, { string.format('MKD %s', ftp_path) }, cb) ftpcmd(res, { string.format('ftp.voidcmd(%q)', 'MKD ' .. ftp_path) }, 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
@ -492,9 +496,9 @@ M.perform_action = function(action, cb)
local res = M.parse_url(action.url) local res = M.parse_url(action.url)
local ftp_path = ftp_abs_path(res) local ftp_path = ftp_abs_path(res)
if action.entry_type == 'directory' then if action.entry_type == 'directory' then
ftpcmd(res, { string.format('RMD %s', ftp_path) }, cb) ftpcmd(res, { string.format('ftp.voidcmd(%q)', 'RMD ' .. ftp_path) }, cb)
else else
ftpcmd(res, { string.format('DELE %s', ftp_path) }, cb) ftpcmd(res, { string.format('ftp.voidcmd(%q)', 'DELE ' .. ftp_path) }, cb)
end end
elseif action.type == 'move' then elseif action.type == 'move' then
local src_adapter = assert(config.get_adapter_by_scheme(action.src_url)) local src_adapter = assert(config.get_adapter_by_scheme(action.src_url))
@ -504,8 +508,7 @@ M.perform_action = function(action, cb)
local dest_res = M.parse_url(action.dest_url) local dest_res = M.parse_url(action.dest_url)
if url_hosts_equal(src_res, dest_res) then if url_hosts_equal(src_res, dest_res) then
ftpcmd(src_res, { ftpcmd(src_res, {
string.format('RNFR %s', ftp_abs_path(src_res)), string.format('ftp.rename(%q, %q)', ftp_abs_path(src_res), ftp_abs_path(dest_res)),
string.format('RNTO %s', ftp_abs_path(dest_res)),
}, cb) }, cb)
else else
if action.entry_type == 'directory' then if action.entry_type == 'directory' then
@ -516,7 +519,11 @@ M.perform_action = function(action, cb)
if err then if err then
return cb(err) return cb(err)
end end
ftpcmd(src_res, { string.format('DELE %s', ftp_abs_path(src_res)) }, cb) ftpcmd(
src_res,
{ string.format('ftp.voidcmd(%q)', 'DELE ' .. ftp_abs_path(src_res)) },
cb
)
end) end)
end end
else else