diff --git a/README.md b/README.md index 77af1b3..4b49e9d 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/doc/arti-pack.txt b/doc/arti-pack.txt index e7b1fd6..841097f 100644 --- a/doc/arti-pack.txt +++ b/doc/arti-pack.txt @@ -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: > ///archive/refs/tags/. diff --git a/lua/arti-pack/spec.lua b/lua/arti-pack/spec.lua index be48769..2bf6f6e 100644 --- a/lua/arti-pack/spec.lua +++ b/lua/arti-pack/spec.lua @@ -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 diff --git a/tests/spec_spec.lua b/tests/spec_spec.lua index 1dcb818..5076f18 100644 --- a/tests/spec_spec.lua +++ b/tests/spec_spec.lua @@ -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()