-
Notifications
You must be signed in to change notification settings - Fork 305
Description
The purpose of ResourceMapper was to put the knowledge for URL-to-file and file-to-URL conversion in one place and one place only. That's why only two methods were exposed: mapUrlToFile and mapFileToUrl, and they were very deeply tested. This avoided a whole range of bugs caused by spaces, escapes, etc. You can see this old state here.
However, later versions started exposing extra methods, such as resolveUrl, getFullPath.
On the one hand, it is uncertain whether these are as well-tested. On the other hand, it leads to modules again rolling their own resolving and introducing errors in the process. @timbl has found cases where URLs suddenly have unescaped spaces and are thus invalid (#1211). In the linked example, we see a bug happening:
// This is a file path (unescaped spaces)
const path = ldp.resourceMapper.getFullPath(req)
// Woops, now suddenly it is a URL (invalid, because unescaped spaces!)
const requestUri = await ldp.resourceMapper.resolveUrl(req.hostname, path)Propose resolution: do not expose resolveUrl and getFullPath (underscore them); only mapUrlToFile and mapFileToUrl should be used.