-
Notifications
You must be signed in to change notification settings - Fork 53
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
Consistancy issues in JSON schema #75
Comments
This is an issue we have encountered aswell |
I was the one that added the schema to the original project, it was added 'after the fact'. Then was modified further after that. Since it was added after the data files existed (not before), it had to conform to the data present after years of different people contributing with no guidance or format restrictions. I agree if it was more strongly typed that would really help. On the off chance that it might help someone here's the parser we use within ZAPs technology detection add-on: https://github.com/zaproxy/zap-extensions/blob/main/addOns/wappalyzer/src/main/java/org/zaproxy/zap/extension/wappalyzer/WappalyzerJsonParser.java |
I made some contribution in order to unify and simplify the schema. |
Thanks. Seems fine on my end. Nice work! |
unsure about what to do with the dom selectors, there are too many variations string string array: object I'd say transform string and string array to the object structure, like "dom": {"form[name='formLogin'][action='login.aspx' i][id='formLogin']": ""}, seems more flexible. However this change might break some implementations so i thought on creating a new one called "selector" and deprecating the "dom" one. |
Well assuming people are already using the objects then moving strings and arrays into objects "shouldn't" break anything ... Though I guess there might be some just using the strings and string array elements 🤷♂️ |
Yeah the thing is i'm not sure what is the people using right now, my guess would be json object aswell, I checked some libs on github and people are not even using the query selectors |
I can say that ZAP uses both but I think our implementation south continue to work. The easy, consistent, and quick'ish thing I guess is to make all the single strings into single element arrays as was done elsewhere. Then atleast it's down to two types vs. three. |
I guess, still not consistent you can't properly map it on a strongly typed language -- but I'm not sure if you could anyways, since it seems to have unknown depth, so... |
there appears to be a limited amout of keys inside the selector
any of the " Of course many if them seem to be very broken so im going to work on them now. But still you can't map it because even tho attributes, text or exists seem to be mandatory, the key that contains it is dynamic, as well as the keys inside the attributes obj... a better way of mapping those objects would be like
so its easier to scale aswell like this
also I'm not sure what the exists is for, since if you dont want it, just dont put it. |
anyways, knowing this structure at least we can properly validate everything, not sure how many people use this objects as they are supposed to since there is no documentation of it. Also many of the objects are broken no idea how this has been working in prod, I've seen at least 5 or 6 randomly structured objects. |
Remember they aren't regex they're element selectors. Our project is using them without issue. |
Oh okay, now I see what you mean |
Describe the bug
The definition of the schema in
schema.json
specifies that some fields may be one of several types.For instance, the field
implies
can either be a string or an array (of strings):This makes it really hard to parse the JSON object in languages like Go where you need to define the types statically:
Trying to deserialize a JSON object into an instance of this struct will return an error if the JSON input is using a string type instead of an array of strings:
Expected behavior
Have a single type for each field.
The text was updated successfully, but these errors were encountered: