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

TM Schema regex error #1906

Closed
egekorkan opened this issue Oct 23, 2023 · 8 comments · Fixed by #1912
Closed

TM Schema regex error #1906

egekorkan opened this issue Oct 23, 2023 · 8 comments · Fixed by #1912
Labels
bug Thing Model Topic related to Thing Models validation Topic related to Normative Parsing, Validation, Consumption

Comments

@egekorkan
Copy link
Contributor

egekorkan commented Oct 23, 2023

In our TM Schema, we have a regex at https://github.com/w3c/wot-thing-description/blob/main/validation/tm-json-schema-validation.json#L1852 . This is sadly failing in golang and rust and subsequently the TM Schema cannot be used by many golang based JSON Schema validators (not sure about Rust yet). An alternative is available at https://regex101.com/r/qSBNcw/1 but that fails for Rust. Ideally we should fix this asap.

Note: / is not reserved character for regex but we are escaping it but I do not remember why

@egekorkan egekorkan added validation Topic related to Normative Parsing, Validation, Consumption Thing Model Topic related to Thing Models labels Oct 23, 2023
@github-actions github-actions bot added the needs-triage Automatically added to new issues. TF should triage them with proper labels label Oct 23, 2023
@egekorkan egekorkan removed the needs-triage Automatically added to new issues. TF should triage them with proper labels label Oct 23, 2023
@egekorkan
Copy link
Contributor Author

Basically, the backreference (\1) is the issue and it is not recommended by https://json-schema.org/understanding-json-schema/reference/regular_expressions as well. They say

complete syntax is not widely supported, therefore it is recommended that you stick to the subset of that syntax described below

@egekorkan
Copy link
Contributor Author

At the same time, Rust does not seem to accept escaping / , i.e. \/ . Any ideas why @lu-zero ?

@lu-zero
Copy link
Contributor

lu-zero commented Oct 25, 2023

The json schema validators I know are;

Which one are you using?

@egekorkan
Copy link
Contributor Author

I am using https://regex101.com/ so not a JSON Schema validator. It seems that JSON Schema libraries offload regex to standard libraries of the language

@lu-zero
Copy link
Contributor

lu-zero commented Oct 25, 2023

rust does not have regex as part of the standard library.

@lu-zero
Copy link
Contributor

lu-zero commented Oct 25, 2023

Both valico and jsonschema-rs rely on fancy-regex to compile the regex and that crate supports pretty much anything the jsonschema specification may allow and some more.

@lu-zero
Copy link
Contributor

lu-zero commented Oct 25, 2023

From a quick look also a nice jsonschema impl in golang seems to be aware of their shortcoming: qri-io/jsonschema#119. So either we make aware our downstreams that we expect that their jsonschema validators support regex-with-backreferences or we have to make sure we do not use those features.

@egekorkan egekorkan added the bug label Oct 25, 2023
@egekorkan
Copy link
Contributor Author

Call of 25.10:

  • @lu-zero : The golang libraries that do not support the syntax should fix themselves, we should not be asked to change or fix implementations. +1 from Mahda
  • @relu91 : We should document the corner case. It would be good to have "works with languages below" table with some notes in the readme. We should make it more correct
  • @egekorkan : We accept too much at the moment as well, e.g. /props/temp is accepted

Decision: Make it more correct. Two regexes that can be combined:

  1. https://regex101.com/r/Ovc1ht/1
  2. https://regex101.com/r/n2HWgd/1

Also relevant for #1749

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Thing Model Topic related to Thing Models validation Topic related to Normative Parsing, Validation, Consumption
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants