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

Nested example - special characters in JSON property names #92

Open
miguelzapaton opened this issue Mar 23, 2021 · 4 comments
Open

Nested example - special characters in JSON property names #92

miguelzapaton opened this issue Mar 23, 2021 · 4 comments

Comments

@miguelzapaton
Copy link
Contributor

Dear Tobias,

Good to see that you're back and at least a little active.

I observed the following misbehavior of VFJS when using a nested JSON schema and parent property names that contain special characters (in my case - and #).

To demonstrate it I forked example 7. The only thing that I changed is that the parent property "user" is named "u#-ser" now:

https://codesandbox.io/s/example-7-forked-ss94p

Please have a look at the bottom of the VFJSstate after submitting the form.
There are strange state properties like "'u#-ser'].consent". That's why the error handling is not working properly.

I had a look at your code but I don't get it out where the problem could be.
It has somehow to do with string handling, probably during deep cloning?

In case that you wouldn't have time I'd be glad to get a small hint where to look at. Then I'll try to prepare a PR with the fix.

Many thanks in advance and good luck!

Miguel

@jarvelov
Copy link
Owner

Hi Miguel!

Thanks for reporting this issue. I haven't encountered this before and it definitely is a bug, that's for sure. The places where I would start to look for this error to originate from would include:

  • There might be some limit Ajv library / JSON Schema specification might not support all characters for key names, so I would check the input and output of Ajv validation, which is what is then set as the field's property value in the schema.
  • Setting of the schema key, which should be done with lodash's set function, it might be just a schema[key] = state assignment, in which case that would be a prime suspect for this issue, and simply replacing the assignment of the key to use lodash's set function might resolve it.
  • The deep cloning you mentioned
  • In the communication between VFJS and its parent component via events / props. There might be some serialization/deserialization which doesn't work properly

I don't have time until this weekend to do some digging into this issue, in the meantime I hope one of the leads I provided might get you somewhere.

@miguelzapaton
Copy link
Contributor Author

Thank you for your fast response!

In the meantime I found out that only parent properties are affected by the strange behavior.

A fork of example 6 with a property named "f#-irstName" works perfectly:

https://codesandbox.io/s/example-6-forked-b0g6z

Manually validating the data with AJV doesn't show any anomalies regarding the dataPath either.

So I will continue investigating your hints 2 - 4 ...

Btw. the examples are still using an older version of VFJS.

Miguel

@miguelzapaton
Copy link
Contributor Author

Solution

Finally I found out what happens:

The problem is related to AJV validation and the returned error object.

There are two cases:

I) The parent property does not contain any special characters

error.dataPath = ".user" -> JSONpath 'normal' notation

I) The parent property does contain any special characters

error.dataPath = "['u#-ser']" -> JSONpath 'bracket' notation

As a consequence the parsing used in:

src/vfjs-global-mixin/methods/vfjs-validation/getters.js

...
const parent = String(error.dataPath).substr(1).replace(/\//g, '.');
....

works only for the 'normal' JSONpath notation.

The solution would be to check if it's either normal or bracket notation and parse it differently.

Additional remarks

During my investigation I found out that the AJV version currently used in VFJS is not the newest version.

There are several breaking changes and one would affect the above mentioned problem and the solution.

Newer versions of AJV use JSONpointer instead of JSONpath for the dataPath (now called instancePath) and schemaPath property. In earlier versions this was also possible by using the AJV option jsonPointers: true.

As JSONpointer doesn't have two types of notation it would make the fix of this problem easier.

In order to avoid further problems with special characters it would be necessary to check if JSONpoint/path use escaping for some chars as this could also break the functionality of VFJS.

Depending on your feedback I can try to create a PR with the preferred solution.

Miguel

@jarvelov
Copy link
Owner

Hi Miguel!

Nice detective work! I think it would make sense to upgrade Ajv to latest version, however if the upgrade is difficult I think we could release a patch version with just the parsing fix, and then in another update include the changes required for the Ajv version upgrade.

I do not know how much work it would require to upgrade Ajv. I should be able to look into this sometime this week, but my gut feeling tells me that it would probably be next week at the earliest that I could release that version even if you submitted a PR for it. The patch version seems like a simple fix and is one I could probably release immediately if you submitted a PR for it.

Again, thanks for looking into this. It is very much appreciated!

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