fix(loader): cache path ambiguity #24491

Problem: cache paths are derived by replacing each reserved/filesystem-
path-sensitive char with a `%` char in the original path. With this
method, two different files at two different paths (each containing `%`
chars) can erroneously resolve to the very same cache path in certain
edge-cases.

Solution: derive cache paths by url-encoding the original (path) instead
using `vim.uri_encode()` with `"rfc2396"`. Increment `Loader.VERSION` to
denote this change.

4 files changed, 99 insertions, 56 deletions

diff --git a/runtime/doc/lua.txt b/runtime/doc/lua.txt
index 96569f8d52..32dcd930bb 100644
--- a/runtime/doc/lua.txt
+++ b/runtime/doc/lua.txt
@@ -2428,8 +2428,27 @@ vim.loader.reset({path})                                  *vim.loader.reset()*
 ==============================================================================
 Lua module: vim.uri                                                  *vim.uri*
 
+vim.uri_decode({str})                                       *vim.uri_decode()*
+    URI-decodes a string containing percent escapes.
+
+    Parameters: ~
+      • {str}  (string) string to decode
+
+    Return: ~
+        (string) decoded string
+
+vim.uri_encode({str}, {rfc})                                *vim.uri_encode()*
+    URI-encodes a string using percent escapes.
+
+    Parameters: ~
+      • {str}  (string) string to encode
+      • {rfc}  "rfc2396" | "rfc2732" | "rfc3986" | nil
+
+    Return: ~
+        (string) encoded string
+
 vim.uri_from_bufnr({bufnr})                             *vim.uri_from_bufnr()*
-    Get a URI from a bufnr
+    Gets a URI from a bufnr.
 
     Parameters: ~
       • {bufnr}  (integer)
@@ -2438,7 +2457,7 @@ vim.uri_from_bufnr({bufnr})                             *vim.uri_from_bufnr()*
         (string) URI
 
 vim.uri_from_fname({path})                              *vim.uri_from_fname()*
-    Get a URI from a file path.
+    Gets a URI from a file path.
 
     Parameters: ~
       • {path}  (string) Path to file
@@ -2447,7 +2466,7 @@ vim.uri_from_fname({path})                              *vim.uri_from_fname()*
         (string) URI
 
 vim.uri_to_bufnr({uri})                                   *vim.uri_to_bufnr()*
-    Get the buffer for a uri. Creates a new unloaded buffer if no buffer for
+    Gets the buffer for a uri. Creates a new unloaded buffer if no buffer for
     the uri already exists.
 
     Parameters: ~
@@ -2457,7 +2476,7 @@ vim.uri_to_bufnr({uri})                                   *vim.uri_to_bufnr()*
         (integer) bufnr
 
 vim.uri_to_fname({uri})                                   *vim.uri_to_fname()*
-    Get a filename from a URI
+    Gets a filename from a URI.
 
     Parameters: ~
       • {uri}  (string)
diff --git a/runtime/lua/vim/loader.lua b/runtime/lua/vim/loader.lua
index 4f4722b0c3..e08ccba701 100644
--- a/runtime/lua/vim/loader.lua
+++ b/runtime/lua/vim/loader.lua
@@ -1,4 +1,5 @@
 local uv = vim.uv
+local uri_encode = vim.uri_encode
 
 --- @type (fun(modename: string): fun()|string)[]
 local loaders = package.loaders
@@ -33,7 +34,7 @@ M.enabled = false
 ---@field _rtp_key string
 ---@field _hashes? table<string, CacheHash>
 local Loader = {
-  VERSION = 3,
+  VERSION = 4,
   ---@type table<string, table<string,ModuleInfo>>
   _indexed = {},
   ---@type table<string, string[]>
@@ -99,7 +100,7 @@ end
 ---@return string file_name
 ---@private
 function Loader.cache_file(name)
-  local ret = M.path .. '/' .. name:gsub('[/\\:]', '%%')
+  local ret = ('%s/%s'):format(M.path, uri_encode(name, 'rfc2396'))
   return ret:sub(-4) == '.lua' and (ret .. 'c') or (ret .. '.luac')
 end
 
diff --git a/runtime/lua/vim/uri.lua b/runtime/lua/vim/uri.lua
index eaa64a6fad..9dce80b77e 100644
--- a/runtime/lua/vim/uri.lua
+++ b/runtime/lua/vim/uri.lua
@@ -1,65 +1,71 @@
---- TODO: This is implemented only for files now.
+---TODO: This is implemented only for files currently.
 -- https://tools.ietf.org/html/rfc3986
 -- https://tools.ietf.org/html/rfc2732
 -- https://tools.ietf.org/html/rfc2396
 
-local uri_decode
-do
-  local schar = string.char
+local M = {}
+local sbyte = string.byte
+local schar = string.char
+local tohex = require('bit').tohex
+local URI_SCHEME_PATTERN = '^([a-zA-Z]+[a-zA-Z0-9.+-]*):.*'
+local WINDOWS_URI_SCHEME_PATTERN = '^([a-zA-Z]+[a-zA-Z0-9.+-]*):[a-zA-Z]:.*'
+local PATTERNS = {
+  ---RFC 2396
+  ---https://tools.ietf.org/html/rfc2396#section-2.2
+  rfc2396 = "^A-Za-z0-9%-_.!~*'()",
+  ---RFC 2732
+  ---https://tools.ietf.org/html/rfc2732
+  rfc2732 = "^A-Za-z0-9%-_.!~*'()[]",
+  ---RFC 3986
+  ---https://tools.ietf.org/html/rfc3986#section-2.2
+  rfc3986 = "^A-Za-z0-9%-._~!$&'()*+,;=:@/",
+}
 
-  --- Convert hex to char
-  local function hex_to_char(hex)
-    return schar(tonumber(hex, 16))
-  end
-  uri_decode = function(str)
-    return str:gsub('%%([a-fA-F0-9][a-fA-F0-9])', hex_to_char)
-  end
+---Converts hex to char
+---@param hex string
+---@return string
+local function hex_to_char(hex)
+  return schar(tonumber(hex, 16))
 end
 
-local uri_encode
-do
-  local PATTERNS = {
-    --- RFC 2396
-    -- https://tools.ietf.org/html/rfc2396#section-2.2
-    rfc2396 = "^A-Za-z0-9%-_.!~*'()",
-    --- RFC 2732
-    -- https://tools.ietf.org/html/rfc2732
-    rfc2732 = "^A-Za-z0-9%-_.!~*'()[]",
-    --- RFC 3986
-    -- https://tools.ietf.org/html/rfc3986#section-2.2
-    rfc3986 = "^A-Za-z0-9%-._~!$&'()*+,;=:@/",
-  }
-  local sbyte = string.byte
-  local tohex = require('bit').tohex
-
-  local function percent_encode_char(char)
-    return '%' .. tohex(sbyte(char), 2)
-  end
-  uri_encode = function(text, rfc)
-    if not text then
-      return
-    end
-    local pattern = PATTERNS[rfc] or PATTERNS.rfc3986
-    return text:gsub('([' .. pattern .. '])', percent_encode_char)
-  end
+---@param char string
+---@return string
+local function percent_encode_char(char)
+  return '%' .. tohex(sbyte(char), 2)
 end
 
+---@param uri string
+---@return boolean
 local function is_windows_file_uri(uri)
   return uri:match('^file:/+[a-zA-Z]:') ~= nil
 end
 
-local M = {}
+---URI-encodes a string using percent escapes.
+---@param str string string to encode
+---@param rfc "rfc2396" | "rfc2732" | "rfc3986" | nil
+---@return string encoded string
+function M.uri_encode(str, rfc)
+  local pattern = PATTERNS[rfc] or PATTERNS.rfc3986
+  return (str:gsub('([' .. pattern .. '])', percent_encode_char)) -- clamped to 1 retval with ()
+end
+
+---URI-decodes a string containing percent escapes.
+---@param str string string to decode
+---@return string decoded string
+function M.uri_decode(str)
+  return (str:gsub('%%([a-fA-F0-9][a-fA-F0-9])', hex_to_char)) -- clamped to 1 retval with ()
+end
 
---- Get a URI from a file path.
+---Gets a URI from a file path.
 ---@param path string Path to file
 ---@return string URI
 function M.uri_from_fname(path)
   local volume_path, fname = path:match('^([a-zA-Z]:)(.*)')
   local is_windows = volume_path ~= nil
   if is_windows then
-    path = volume_path .. uri_encode(fname:gsub('\\', '/'))
+    path = volume_path .. M.uri_encode(fname:gsub('\\', '/'))
   else
-    path = uri_encode(path)
+    path = M.uri_encode(path)
   end
   local uri_parts = { 'file://' }
   if is_windows then
@@ -69,10 +75,7 @@ function M.uri_from_fname(path)
   return table.concat(uri_parts)
 end
 
-local URI_SCHEME_PATTERN = '^([a-zA-Z]+[a-zA-Z0-9.+-]*):.*'
-local WINDOWS_URI_SCHEME_PATTERN = '^([a-zA-Z]+[a-zA-Z0-9.+-]*):[a-zA-Z]:.*'
-
---- Get a URI from a bufnr
+---Gets a URI from a bufnr.
 ---@param bufnr integer
 ---@return string URI
 function M.uri_from_bufnr(bufnr)
@@ -93,7 +96,7 @@ function M.uri_from_bufnr(bufnr)
   end
 end
 
---- Get a filename from a URI
+---Gets a filename from a URI.
 ---@param uri string
 ---@return string filename or unchanged URI for non-file URIs
 function M.uri_to_fname(uri)
@@ -101,8 +104,8 @@ function M.uri_to_fname(uri)
   if scheme ~= 'file' then
     return uri
   end
-  uri = uri_decode(uri)
-  -- TODO improve this.
+  uri = M.uri_decode(uri)
+  --TODO improve this.
   if is_windows_file_uri(uri) then
     uri = uri:gsub('^file:/+', '')
     uri = uri:gsub('/', '\\')
@@ -112,8 +115,8 @@ function M.uri_to_fname(uri)
   return uri
 end
 
---- Get the buffer for a uri.
---- Creates a new unloaded buffer if no buffer for the uri already exists.
+---Gets the buffer for a uri.
+---Creates a new unloaded buffer if no buffer for the uri already exists.
 --
 ---@param uri string
 ---@return integer bufnr
diff --git a/test/functional/lua/loader_spec.lua b/test/functional/lua/loader_spec.lua
index e2958d1592..34c36b04ef 100644
--- a/test/functional/lua/loader_spec.lua
+++ b/test/functional/lua/loader_spec.lua
@@ -33,4 +33,24 @@ describe('vim.loader', function()
       return _G.TEST
     ]], tmp))
   end)
+
+  it('handles % signs in modpath (#24491)', function()
+    exec_lua[[
+      vim.loader.enable()
+    ]]
+
+    local tmp1, tmp2 = (function (t)
+      assert(os.remove(t))
+      assert(helpers.mkdir(t))
+      assert(helpers.mkdir(t .. '/%'))
+      return t .. '/%/x', t .. '/%%x'
+    end)(helpers.tmpname())
+
+    helpers.write_file(tmp1, 'return 1', true)
+    helpers.write_file(tmp2, 'return 2', true)
+    vim.uv.fs_utime(tmp1, 0, 0)
+    vim.uv.fs_utime(tmp2, 0, 0)
+    eq(1, exec_lua('return loadfile(...)()', tmp1))
+    eq(2, exec_lua('return loadfile(...)()', tmp2))
+  end)
 end)