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

Nodes containing / #169

Open
mbtools opened this issue Aug 9, 2023 · 3 comments · May be fixed by #193
Open

Nodes containing / #169

mbtools opened this issue Aug 9, 2023 · 3 comments · May be fixed by #193

Comments

@mbtools
Copy link
Contributor

mbtools commented Aug 9, 2023

I'm trying to parse https://raw.githubusercontent.com/abapedia/steampunk-2305-api/main/src/_status.json with AJSON.

{
  "clas,/dmo/cx_rap_generator": {
    "status": "RELEASED",
    "successors": []
  },
  "clas,/iwbep/cl_cp_factory_remote": {
    "status": "RELEASED",
    "successors": []
  },
  "clas,/iwbep/cl_cp_factory_unit_tst": {
    "status": "RELEASED",
    "successors": []
  },

There are no errors but the result is incorrect (the path column):

image

Clearly the / in the name confuses AJSON and should be escaped. Not sure how you want to approach a fix here.

@sbcgua
Copy link
Owner

sbcgua commented Aug 9, 2023

Yeah ... that can ruin some internals ... especially UTs ...

The first link is a bit incorrect. Not all. All but the control chars. They must be escaped. Meaning that they will not present in a string. So the first idea is to use tab. But it will ruin all UTs and actually all existing addressing. Maybe in opposite, the / in the key should be replaced with tab ? Let's think about it for a while.

@mbtools
Copy link
Contributor Author

mbtools commented Aug 9, 2023

You are right, the / should be escaped in the original JSON. Maybe best to raise an error if the value contains unescaped special characters. But I guess it's still a problem if the value contains \/ (haven't tried).

@sbcgua
Copy link
Owner

sbcgua commented Aug 10, 2023

That means unsupporting a valid JSON ... not ideal. I'm thinking on autoreplacing / with a tab for a path before search/set/etc ... but this is also a performance impact :(

mbtools added a commit to mbtools/ajson that referenced this issue 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 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 mbtools linked a pull request Dec 6, 2024 that will close this issue
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 a pull request may close this issue.

2 participants