Trim whitespace out of the registry and spec urls
A registry with a stray space or newline passed validation and was concatenated verbatim into the archive URL, so curl saw the malformed address instead of the user seeing the required-registry error. Trim the value before matching and reject any whitespace left inside it; apply the same to a spec's explicit url.
This commit is contained in:
@@ -41,7 +41,9 @@ require('arti-pack').setup({
|
||||
|
||||
`registry` is required and has no default: it is the URL prefix your own
|
||||
artifact repository serves upstream tag archives under, so there is no sensible
|
||||
value to ship. Until you set it, `:ArtiPackSync` refuses to run and says so:
|
||||
value to ship. It must be an `http://` or `https://` URL; surrounding
|
||||
whitespace is trimmed and anything else is rejected. Until you set it,
|
||||
`:ArtiPackSync` refuses to run and says so:
|
||||
|
||||
```
|
||||
[arti-pack] registry is not configured; set it in require('arti-pack').setup{ registry = 'https://artifacts.example.com/artifactory/github-remote' }
|
||||
@@ -63,7 +65,7 @@ Every plugin needs an explicit `tag` — that is the point. Plugins install to
|
||||
| `opt` | `false` | Install under `opt/` for `:packadd` instead of `start/` |
|
||||
| `as` | repo name | Directory name to install as |
|
||||
| `ext` | `'zip'` | `zip` or `tar.gz` |
|
||||
| `url` | — | Full archive URL, bypassing the registry |
|
||||
| `url` | — | Full archive URL, bypassing the registry (no whitespace) |
|
||||
|
||||
### Options
|
||||
|
||||
|
||||
+9
-6
@@ -40,11 +40,13 @@ Options:
|
||||
|
||||
registry Required, no default. Archive URL prefix placed in front of
|
||||
"owner/repo", pointing at your own artifact repository. It
|
||||
must be an http:// or https:// URL; anything else is
|
||||
rejected at setup. Until it is set, |:ArtiPackSync| refuses
|
||||
to run and says which option to set. |:ArtiPackClean| and
|
||||
|:ArtiPackList| never resolve a URL and work without it, as
|
||||
do plugin specs carrying an explicit "url".
|
||||
must be an http:// or https:// URL; surrounding whitespace
|
||||
is trimmed and anything else is rejected at setup, so a
|
||||
stray space never reaches the download. Until it is set,
|
||||
|:ArtiPackSync| refuses to run and says which option to
|
||||
set. |:ArtiPackClean| and |:ArtiPackList| never resolve a
|
||||
URL and work without it, as do plugin specs carrying an
|
||||
explicit "url".
|
||||
plugins List of plugin specs. See |arti-pack-specs|.
|
||||
auto_sync Sync missing and changed plugins on startup. Default true.
|
||||
concurrency Plugins downloaded in parallel. Default 4.
|
||||
@@ -59,7 +61,8 @@ Each entry is a table whose first element is "owner/repo".
|
||||
opt Install under opt/ for |:packadd| instead of start/. Default false.
|
||||
as Directory name to install as. Defaults to the repo name.
|
||||
ext "zip" or "tar.gz". Default "zip".
|
||||
url Full archive URL, bypassing the registry.
|
||||
url Full archive URL, bypassing the registry. Trimmed; it may not
|
||||
contain whitespace.
|
||||
|
||||
The archive URL is built as: >
|
||||
<registry>/<owner>/<repo>/archive/refs/tags/<tag>.<ext>
|
||||
|
||||
+21
-8
@@ -51,8 +51,15 @@ function M.parse(entry)
|
||||
return nil, string.format('%s resolves to an unusable directory name %q', source, tostring(name))
|
||||
end
|
||||
|
||||
if entry.url ~= nil and type(entry.url) ~= 'string' then
|
||||
return nil, string.format('%s has a non-string url', source)
|
||||
local url = entry.url
|
||||
if url ~= nil then
|
||||
if type(url) ~= 'string' then
|
||||
return nil, string.format('%s has a non-string url', source)
|
||||
end
|
||||
url = trim(url)
|
||||
if url == '' or url:find('%s') then
|
||||
return nil, string.format('%s has an invalid url %q', source, entry.url)
|
||||
end
|
||||
end
|
||||
|
||||
return {
|
||||
@@ -63,21 +70,27 @@ function M.parse(entry)
|
||||
tag = tag,
|
||||
ext = ext,
|
||||
opt = entry.opt == true,
|
||||
url = entry.url,
|
||||
url = url,
|
||||
}
|
||||
end
|
||||
|
||||
-- There is no default registry: the URL is site-specific, and a wrong one only
|
||||
-- surfaces as an opaque curl failure. Returns the base with trailing slashes
|
||||
-- stripped so joining it to a path never produces "//".
|
||||
-- surfaces as an opaque curl failure. Returns the base trimmed and with trailing
|
||||
-- slashes stripped so joining it to a path never produces "//".
|
||||
function M.check_registry(registry)
|
||||
if registry == nil then
|
||||
return nil, 'registry is not configured; ' .. SETUP_HINT
|
||||
end
|
||||
if type(registry) ~= 'string' or not registry:match('^https?://[^%s/]') then
|
||||
return nil, string.format('registry %s is not an http(s) URL; %s', vim.inspect(registry), SETUP_HINT)
|
||||
local bad = string.format('registry %s is not an http(s) URL; %s', vim.inspect(registry), SETUP_HINT)
|
||||
if type(registry) ~= 'string' then
|
||||
return nil, bad
|
||||
end
|
||||
return (registry:gsub('/+$', ''))
|
||||
-- Whitespace left inside the URL would be baked into every download.
|
||||
local base = trim(registry)
|
||||
if not base:match('^https?://[^%s/]') or base:find('%s') then
|
||||
return nil, bad
|
||||
end
|
||||
return (base:gsub('/+$', ''))
|
||||
end
|
||||
|
||||
-- Tagged source archives live at the same path shape on every GitHub-alike, so
|
||||
|
||||
@@ -76,6 +76,19 @@ t.describe('spec.parse', function()
|
||||
t.eq('owner/repo', s.source)
|
||||
t.eq('v1.0.0', s.tag)
|
||||
end)
|
||||
|
||||
t.it('trims surrounding whitespace from an explicit url', function()
|
||||
local s = assert(spec.parse({ 'owner/repo', tag = 'v1', url = ' https://example.test/a.zip\n' }))
|
||||
t.eq('https://example.test/a.zip', s.url)
|
||||
end)
|
||||
|
||||
t.it('rejects a url with whitespace inside it', function()
|
||||
for _, bad in ipairs({ 'https://example.test/a b.zip', 'https://example\n.test/a.zip', ' ' }) do
|
||||
local s, err = spec.parse({ 'owner/repo', tag = 'v1', url = bad })
|
||||
t.falsy(s, 'rejects ' .. vim.inspect(bad))
|
||||
t.matches('invalid url', err)
|
||||
end
|
||||
end)
|
||||
end)
|
||||
|
||||
t.describe('spec.url', function()
|
||||
@@ -94,6 +107,20 @@ t.describe('spec.url', function()
|
||||
t.eq(REGISTRY .. '/owner/repo/archive/refs/tags/v1.zip', spec.url(s, REGISTRY .. '///'))
|
||||
end)
|
||||
|
||||
t.it('never builds a url containing whitespace', function()
|
||||
local s = assert(spec.parse({ 'owner/repo', tag = 'v1' }))
|
||||
local want = REGISTRY .. '/owner/repo/archive/refs/tags/v1.zip'
|
||||
t.eq(want, spec.url(s, REGISTRY .. ' '))
|
||||
t.eq(want, spec.url(s, REGISTRY .. '\n'))
|
||||
t.eq(want, spec.url(s, ' ' .. REGISTRY))
|
||||
|
||||
for _, bad in ipairs({ 'https://arti facts.example.com/gh', 'https://artifacts\n.example.com/gh', ' ' }) do
|
||||
local url, err = spec.url(s, bad)
|
||||
t.falsy(url, 'rejects ' .. vim.inspect(bad))
|
||||
t.matches('is not an http%(s%) URL', err)
|
||||
end
|
||||
end)
|
||||
|
||||
t.it('passes an explicit url straight through', function()
|
||||
local s = assert(spec.parse({ 'owner/repo', tag = 'v1', url = 'https://example.test/a.zip' }))
|
||||
t.eq('https://example.test/a.zip', spec.url(s, REGISTRY))
|
||||
@@ -127,6 +154,32 @@ t.describe('spec.check_registry', function()
|
||||
t.it('accepts a plain http registry', function()
|
||||
t.eq('http://artifacts.example.com/gh', spec.check_registry('http://artifacts.example.com/gh'))
|
||||
end)
|
||||
|
||||
t.it('trims leading whitespace', function()
|
||||
t.eq(REGISTRY, spec.check_registry(' ' .. REGISTRY))
|
||||
end)
|
||||
|
||||
t.it('trims trailing whitespace, including a newline', function()
|
||||
t.eq(REGISTRY, spec.check_registry(REGISTRY .. ' '))
|
||||
t.eq(REGISTRY, spec.check_registry(REGISTRY .. '\n'))
|
||||
t.eq(REGISTRY, spec.check_registry(' \t' .. REGISTRY .. '/ \n'))
|
||||
end)
|
||||
|
||||
t.it('rejects whitespace embedded in the registry', function()
|
||||
for _, bad in ipairs({ 'https://artifacts.example.com/g h', 'https://artifacts\n.example.com/gh' }) do
|
||||
local base, err = spec.check_registry(bad)
|
||||
t.falsy(base, 'rejects ' .. vim.inspect(bad))
|
||||
t.matches('is not an http%(s%) URL', err)
|
||||
end
|
||||
end)
|
||||
|
||||
t.it('rejects a whitespace-only registry', function()
|
||||
for _, bad in ipairs({ ' ', ' ', '\n', ' \t\n ' }) do
|
||||
local base, err = spec.check_registry(bad)
|
||||
t.falsy(base, 'rejects ' .. vim.inspect(bad))
|
||||
t.matches('is not an http%(s%) URL', err)
|
||||
end
|
||||
end)
|
||||
end)
|
||||
|
||||
t.describe('spec.parse_all', function()
|
||||
|
||||
Reference in New Issue
Block a user