Repository navigation
Paths for new resources are incorrectly generated by LDP POST #1228
Description
When trying to create a new folder via the solid-filemanager I get a 500 - Path must start with hostname (/otman.inrupt.net) with the new version on inrupt.net
I've made a POST to https://otman.inrupt.net/public with slug: my-folder, link: <http://www.w3.org/ns/ldp#BasicContainer>; rel="type" and Content-Type: text/turtle.
It responded with the status 500 and the statusText Path must start with hostname (/otman.inrupt.net).
Is this something we should change in the request setup or an error inside NSS?
Here are the complete request headers (except for bearer token and connect.sid cookie):
POST /public HTTP/1.1
Host: otman.inrupt.net
User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:67.0) Gecko/20100101 Firefox/67.0
Accept: */*
Accept-Language: en,en-US;q=0.7,de;q=0.3
Accept-Encoding: gzip, deflate, br
link: <http://www.w3.org/ns/ldp#BasicContainer>; rel="type"
slug: my-folder
Content-Type: text/turtle
authorization: Bearer {...}
Origin: https://solid-filemanager.solid.community
Content-Length: 0
DNT: 1
Connection: keep-alive
Referer: https://solid-filemanager.solid.community/public/nightly/?url=https://otman.inrupt.net/public
Cookie: connect.sid=s%3 { ... }
Pragma: no-cache
Cache-Control: no-cache
And the response headers:
HTTP/1.1 500 Internal Server Error
X-Powered-By: solid-server/5.1.5
Access-Control-Allow-Origin: https://solid-filemanager.solid.community
Vary: Accept, Authorization, Origin
Access-Control-Allow-Credentials: true
Access-Control-Expose-Headers: Authorization, User, Location, Link, Vary, Last-Modified, ETag, Accept-Patch, Accept-Post, Updates-Via, Allow, WAC-Allow, Content-Length, WWW-Authenticate
Allow: OPTIONS, HEAD, GET, PATCH, POST, PUT, DELETE
Link: <public.acl>; rel="acl", <public.meta>; rel="describedBy", <http://www.w3.org/ns/ldp#Resource>; rel="type"
Content-Type: text/plain; charset=utf-8
Content-Length: 50
ETag: W/"32-fmcIjz1GGt/H6AvpCi6o3nXP7kw"
Set-Cookie: connect.sid=s%3 {...}; Domain=.inrupt.net; Path=/; Expires=Sun, 16 Jun 2019 05:44:04 GMT; HttpOnly; Secure
Date: Sat, 15 Jun 2019 05:44:04 GMT
Connection: keep-alive
Activity
jaxoncreed commentedon Jun 17, 2019
I believe I know what's causing this @Otto-AA . This is probably a good first issue to get involved with the Node Solid Server maintenance team.
If you're interested, I'd love to hop on a call with you so we can fix it together!
Otto-AA commentedon Jun 17, 2019
I would really love to get more involved into this project, but I am too occupied by other projects and work currently. Therefore I have to gratefully decline your offer for now. If I find more time in the future I will maybe come back to this project though.
michielbdejong commentedon Jun 20, 2019
Was able to reproduce this on dev.inrupt.net with:
(you can get a valid cookie by logging in to the data browser, going to the network pane in the dev tools, and selecting 'copy as curl' on one of the xhr requests)
michielbdejong commentedon Jun 20, 2019
In fact it also happens if you leave out the
-H 'slug: my-folder'michielbdejong commentedon Jun 20, 2019
@RubenVerborgh could this have been caused your recent refactor?
RubenVerborgh commentedon Jun 20, 2019
No refactor happened, but yes, a fix for one bug was added (#1227).
The fix itself is probably not faulty; rather, the code calling this function was (and has always been) at fault, but it went undetected before the fix. Would need a stack trace to pinpoint.
michielbdejong commentedon Jun 20, 2019
I was referring to the ResourceMapper refactor (#1213), which we know also caused the bug that #1227 fixed.
The following suggests this is indeed a problem with the ResourceMapper:
To reproduce on localhost, run in multi-user mode (need to run
./bin/solid-test init), start the server, then visit https://localhost:8443, register a user, for instance 'michielbdejong', add e.g. '127.0.0.1 michielbdejong.localhost' in your /etc/hosts, then do:~/gh/solid/node-solid-server $ curl -k -X POST https://michielbdejong.localhost:8443/inbox/ Path must start with hostname (/michielbdejong.localhost)michielbdejong commentedon Jun 20, 2019
Maybe we should just revert to the code from before #1213?
RubenVerborgh commentedon Jun 20, 2019
No. See above answer.
It shows that the error is thrown by ResourceMapper. ResourceMapper will correctly throw that error when called with the wrong arguments. Hence my request for a stack trace.
michielbdejong commentedon Jun 20, 2019
solid:server Error page because of: Error: Path must start with hostname (/michielbdejong.localhost) at ResourceMapper.mapFileToUrl (/Users/michiel/gh/solid/node-solid-server/lib/resource-mapper.js:74:15) at LDP.post (/Users/michiel/gh/solid/node-solid-server/lib/ldp.js:157:68) +1msmichielbdejong commentedon Jun 20, 2019
lib/ldp.jsis callingthis.resourceMapper.mapFileToUrlwith:michielbdejong commentedon Jun 20, 2019
The "do not expose root path" refactor changed that
resourcePathvalue there fromthis.resourceMapper.rootPath + resourcePathto justresourcePath, so looks like that's triggering the 'Path must start with hostname' error.RubenVerborgh commentedon Jun 20, 2019
Indeed. And in doing so, it is catching a bug that was present in the original code, but went unnoticed. So the error is doing its job 🙂
Historical note:
ResouceMapperwas introduced a year ago, as a way to finally have all knowledge about URL/filesystem correspondence in one place, as a solution to it being implemented 20 times in 10 different and incompatible ways.However,
ResourceMapperwas then extended to leak its abstraction, such that the supposedly internal knowledge was once again spreading through the code. #1213 restored the original intention.Now the change from
this.resourceMapper.rootPath + resourcePathtoresourcePathwas correct (given the updated method): the (semantically) equivalent parameters are ultimately passed to the method. It is just that the original code was wrong in the first place, but this went unnoticed because there was no path correctness check—and now there is.Let me show where it goes wrong:
this.resourceMapper.rootPath + resourcePathwill result in/var/solid/data+/inbox/afaf3830-9355-11e9-850a-2f6182efecf6and thus/var/solid/data/inbox/afaf3830-9355-11e9-850a-2f6182efecf6instead of the correct (assuming multi-user mode)/var/solid/data/michielbdejong.localhost/inbox/afaf3830-9355-11e9-850a-2f6182efecf6. This is precisely the error that is being thrown now, as opposed to going silent.So if this code was actually used as above, then I expect to see lots of random folders under
/var/solid/dataon different servers (ifmkdir -pwas ultimately used); and/or a privilege escalation security bug where people could write to random people's folders by sending aPOSTto/michielbdejong.localhost/private/(let's hope this did not happen).All of the above showing why one component and one component only should implement file to URL mapping.
Now the question, how to fix this bug?
Looking at the code it seems that the call to
mapFileToUrlis wrong to begin with, because there never was a file in the first place. Instead, it should probably be something likeurl.resolve(container, resourcePath)(assuming here thatcontaineris the URL of the container, not sure).RubenVerborgh commentedon Jun 20, 2019
(Not to mention that the
contentTypehack is terrible too.)RubenVerborgh commentedon Jun 20, 2019
@jaxoncreed Key to getting this right will be clearly differentiating between what is a file path (naming suggestion: end with
path, do unsure if properly applied in existing code), and what is a URL. Code should eitherpath.resolvefiles against existing folders,url.resolveURLs against existing URLs, or take absolute file paths or URLs and pass them toResourceMapper. All else will lead to bugs.[-]500 - Path must start with hostname[/-][+]Paths for new resources are incorrectly generated by LDP POST[/+]Merge pull request #1241 from solid/fix/#1228
jaxoncreed commentedon Jun 28, 2019
This should be fixed