nodeSolidServer / node-solid-server

Solid server on top of the file-system in NodeJS
https://solidproject.org/for-developers/pod-server
Other
1.77k stars 297 forks source link

PUT can hide /profile/card with intermediary containers #1743

Closed Otto-AA closed 5 months ago

Otto-AA commented 8 months ago

Problem

When (accidentically) PUTting /profile/card/somedocument, NSS will automatically create the intermediary container /profile/card/. This will result in a situation, where /profile/ contains both the document /profile/card and the folder /profile/card/. See eg https://penny.vincenttunru.com/explore/?url=https%3A%2F%2Fthispoddoesnotexist.solidcommunity.net%2Fprofile%2F

From here on, requests to the WebId will always be redirected to the /profile/card/ folder. You can test this by clicking on the resources in penny, or manually opening them in the browser.

First seen here: https://forum.solidproject.org/t/working-with-multiple-pod-providers/7015/14

Reproduction

  1. Create a pod
  2. Login with the normal mashlib UI
  3. Execute this PUT request in the browser console (or do the PUT request with a different tool):
await window.UI.authn.authSession.fetch('https://thispoddoesnotexist.solidcommunity.net/profile/card/recipes', {
  method: 'PUT',
  headers: { 'Content-Type': 'text/plain' },
  body: 'Pasta + Salsa',
})

Fix suggestion

I'm not sure if the specification covers this case. I guess the PUT should fail with a conflict, if an intermediary container would have the same name as an already existing resource.

Feel free to delete the pod I've used for testing, or just leave it. There is no other data on it :)

zg009 commented 8 months ago

Is this issue still open or was it fixed? I'm working on tests with ldp.put and it is sending back the error that a container and resource cannot contain the same URI, so is there something I am missing?

bourgeoa commented 8 months ago

The issue is still open. It does check the URL but not check the last non existing container when there are more then one level.

If you want the create : profile/card/test

zg009 commented 7 months ago

I have been testing the ldp.put function and is it possible there is an issue with the ResourceMapper class? If I have the test case below:

await ldp.put('profile/card', stringToStream("Pasta + Salad"), 'text/plain');  
await ldp.put('profile/card/test', stringToStream("Pasta + Salad"), 'text/plain');  

then the resources are created. However if I create the test function with prepended slashes like below

await ldp.put('/profile/card', stringToStream("Pasta + Salad"), 'text/plain');  
await ldp.put('/profile/card/test', stringToStream("Pasta + Salad"), 'text/plain');  

then the PUTs fail.

bourgeoa commented 7 months ago

then the resources are created. However if I create the test function with prepended slashes like below

This one should be created. yes

await ldp.put('profile/card', stringToStream("Pasta + Salad"), 'text/plain');  

but not the next one that should fail

await ldp.put('profile/card/test', stringToStream("Pasta + Salad"), 'text/plain');

I don't thing that a /profile is valid . This is not URL. The resourceMapper has been tested for quite along time and in full detail.

zg009 commented 7 months ago

I understand the next one should fail, however when I am checking the resolved Urls returned by the ResourceMapper class using the ldp-test configurations, the urls are including the port number in the first part of the path.

For example: I pass in https://localhost:8443/ as the resource Mapper root url. I pass 'profile/card' as the pathname for ldp.put, and the resourceMapper.resolveUrl is returning https://localhost:8443profile/card, which then accumulates into the next checkFileName function as https://localhost:8443/:8443/profile/ .

This is making the test difficult because this is not expected behavior. The solution I am trying to implement is if the itemUrl ends with a slash, remove the slash, then check if that resource already exists using the resourceMapper.mapUrlToFile method. If it does, fail and don't create a container.

bourgeoa commented 5 months ago

closed with https://github.com/nodeSolidServer/node-solid-server/pull/1750