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

Property names with '/' character cannot be retrieved using path #167

Closed
panick-carousel opened this issue Feb 18, 2022 · 2 comments
Closed

Comments

@panick-carousel
Copy link

panick-carousel commented Feb 18, 2022

Thank you for this library - I appreciate that this problem is very rare.
In this file:

get (path = '/') {
the get function fails if any property keys contain a '/'.

eg: for the object { 'this/will/be/funny': false } if a block is made out of it, block.get('this/will/be/funny') will not work. I think that this library should change the interface to be get( ['this/will/be/funny'] ) instead of a string, to make this unambiguous, but I do appreciate this issue may be too pedantic to warrant a change.

As a curious piece of trivia, linux paths cannot have the '/' in directory names not because of any config or other configurable parameter, but because it remains to this day hard coded into the kernel. JS objects on the other hand....

@rvagg
Copy link
Member

rvagg commented Feb 18, 2022

This is one of those areas where the behaviour is not clearly defined, but it's also difficult to handle. Generally you're advised to avoid such property names because even if we had an elegant means of addressing it here, it's going to appear higher up the stack.

For instance, go-ipld-prime can deal with it because it has a notion of a PathSegment, whereby you can programmatically define a path by segments and the separator is just a formality for presentation: https://github.com/ipld/go-ipld-prime/blob/adcfa4cb3badc359a42721a9958afec11c54a84f/datamodel.go#L84-L86

But that doesn't really help when go-ipfs wants to turn ipfs dag get /path/to/thing because it still has to perform a transformation of that path string to be segments by splitting on /.

If you want to propose an API addition that gives us a clear array-of-segments approach then we could probably work to get that in and give people an avenue to work around the / namespace conflicts; but it's not going to be a universal solution unfortunately.

See also https://ipld.io/docs/data-model/pathing/#path-encoding for more discussion on this as a bit of an unsolved problem.

@panick-carousel
Copy link
Author

Thanks for the detailed response. It sounds like more confusion to users than dealing with the odd person who wants a "/" in their object keys 🤷

@rvagg rvagg mentioned this issue Sep 6, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants