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

Special characters (also nodes containing /) #193

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mbtools
Copy link
Contributor

@mbtools mbtools commented Dec 6, 2024

I added tests for handling of special characters - like slashes, quotes, and unicode - in element names, paths, and values. Everything - except of one case - was working but now we know for sure 😃

The special case is a slash in an element name that has sub-objects: "contains/slash": "test" is ok, but "contains/slash": { "test": 1 } is not. As suggested in #169, I replaced the slash with a tab. Parsing now works well 👍

For methods that require a path as input, we could either leave it as is and remember to use a tab there, too, or we pre-process the input.

* { "contains/slash": { "test": 1 } } or better { "contains\/slash": { "test": 1 } }

r->get( '/contains/slash/test' ). " never worked

r->get( |/contains\tslash/test| ). " works with this PR

r->get( '/contains\/slash/test' ). " could work if the parameter is preprocessed i.e. replace \/ with \t

I would go for the middle case and not worry about the parameters. It will be a rare case anyway.

Closes #169.

I added tests for handling of special characters - like slashes, quotes, and unicode - in element names, paths, and values. Everything - except of one case - was working but now we know for sure.

The special case is a slash in an element name that has sub-objects: `"contains/slash": "test"` is ok, but `"contains/slash": { "test": 1 }` is not. As suggested in sbcgua#169, I replaced the slash with a `tab`. Parsing now works well.

For methods that require a path as input, we could either leave it as is and remember to use a tab there, too, or we pre-process the input. 

```
{ "contains/slash": { "test": 1 } }

r->get( '/contains/slash/test' ). " never worked

r->get( |/contains\tslash/test| ). " works with this PR

r->get( '/contains\/slash/test' ). " could work if the parameter is preprocessed i.e. replace \/ with \t
```

I would go for the middle case and not worry about the parameters. 

Closes sbcgua#169.
@mbtools
Copy link
Contributor Author

mbtools commented Dec 6, 2024

I'm not sure how you want to deal with the lint issue. It's the nature of the test to include Unicode characters. Add it as an exception?

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

Successfully merging this pull request may close these issues.

Nodes containing /
1 participant