Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Otto-AA opened this issue Oct 30, 2023 · 6 comments
Closed

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

Otto-AA opened this issue Oct 30, 2023 · 6 comments
Assignees
Labels

Comments

@Otto-AA
Copy link
Contributor

Otto-AA commented Oct 30, 2023

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
Copy link
Contributor

zg009 commented Nov 14, 2023

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
Copy link
Member

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

  • the check is made on profile/card/test
  • it should be made on last non existing level which in this case is profile/card/ and fail because profile/card exists

@zg009
Copy link
Contributor

zg009 commented Nov 14, 2023

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
Copy link
Member

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
Copy link
Contributor

zg009 commented Nov 16, 2023

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 bourgeoa self-assigned this Dec 19, 2023
@bourgeoa bourgeoa added the bug label Dec 19, 2023
@bourgeoa
Copy link
Member

closed with #1750

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants