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

getResources can include non-child resources #634

Closed
Otto-AA opened this issue Aug 4, 2023 · 1 comment · Fixed by #653
Closed

getResources can include non-child resources #634

Otto-AA opened this issue Aug 4, 2023 · 1 comment · Fixed by #653
Labels
bug Something isn't working

Comments

@Otto-AA
Copy link

Otto-AA commented Aug 4, 2023

Describe the bug

In PR #571 , a validation was added to exclude non-child resources, however the validation misses some edge cases.

Expected behavior

getResources() from a container /foo/ should not include /foo/#bar as a child (and anything else that is not a direct child resource).

Actual behavior

It includes /foo/#bar as a child

How to reproduce?

I've tested it with these lines in container.ttl and the unit tests:

# These containment triples should not be included in a getResources response
<>
    ldp:contains <https://example.com/other> , <newContainer/child> , <> , <./>, <child/..>, <#hashtag>, <../> .

The test case failed because the hashtag was not filtered:

[ERROR]   SolidClientTest.testLowLevelSolidContainer » Completion org.opentest4j.AssertionFailedError: expected: <[http://localhost:36023/container/newContainer/, http://localhost:36023/container/test.txt, http:
//localhost:36023/container/test2.txt]> but was: <[http://localhost:36023/container/newContainer/, http://localhost:36023/container/test.txt, http://localhost:36023/container/test2.txt, http://localhost:36023/co
ntainer/#hashtag]>

Output of java -version

No response

Maven version

No response

Additional information

Cause

The problem likely is, that in the verifyContainmentIri method, the normalizedPath.isEmpty() is called as "#hashtag".isEmpty(). In other words, it thinks that the "#hashtag" is a child resource, rather than just a hashtag.

Suggested fix

I'm not aware how to do this in Java, but I'd leave the path parsing to a library that already has all the edge cases built in (hashtag, query params, and not sure if there's more). Similar to how in solid-client-js they use the built-in child.pathname rather than manually parsing the path segments.

@Otto-AA Otto-AA added the bug Something isn't working label Aug 4, 2023
@NSeydoux
Copy link
Contributor

NSeydoux commented Aug 4, 2023

Hi @Otto-AA,
Thanks for reporting this, we will be looking into a fix shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants