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

added ipv4 and ipv6 validation #843

Merged
merged 5 commits into from
Feb 23, 2023

Conversation

msivasubramaniaan
Copy link
Contributor

Signed-off-by: msivasubramaniaan msivasub@redhat.com

The existing PR has some issues due to my local reabase, Hence created a new one

What does this PR do?

Added IPv4 and IPv6 format validation on parser

What issues does this PR fix or reference?

redhat-developer/vscode-yaml#832

Is it tested? How?

Yes added UT

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@msivasubramaniaan msivasubramaniaan self-assigned this Feb 13, 2023
@msivasubramaniaan msivasubramaniaan linked an issue Feb 13, 2023 that may be closed by this pull request
4 tasks
@coveralls
Copy link

coveralls commented Feb 13, 2023

Coverage Status

Coverage: 83.243% (+0.2%) from 83.063% when pulling 2fea951 on 832-format-ipv4-is-not-being-checked into 9d71bbf on main.

@rgrunber
Copy link
Member

rgrunber commented Feb 22, 2023

When I have something like :

{
    "type": "array",
    "items": {
      "type": "string",
      "format": "ipv6"
    }
}
[
"2001:0db8:85a3:0000:0000:8a2e:0370:7334",
"2001:0db8:85a3:0000:0000:8a2e:0370:7334"
]

and associate the schema with the file, one of the entires doesn't validate.

Update: This is definitely a bug. In fact the root cause is the regular expression used for the ipv4/ipv6 patterns. You've included a flag for your particular pattern that will results in some unpredictable behaviour.

@msivasubramaniaan
Copy link
Contributor Author

msivasubramaniaan commented Feb 23, 2023

When I have something like :

{
    "type": "array",
    "items": {
      "type": "string",
      "format": "ipv6"
    }
}
[
"2001:0db8:85a3:0000:0000:8a2e:0370:7334",
"2001:0db8:85a3:0000:0000:8a2e:0370:7334"
]

and associate the schema with the file, one of the entires doesn't validate.

Update: This is definitely a bug. In fact the root cause is the regular expression used for the ipv4/ipv6 patterns. You've included a flag for your particular pattern that will results in some unpredictable behaviour.

@rgrunber
Agree. That happened due to multi-line flag. Updated the regex pattern and added all the scenarios on the test case for better coverage.
image

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
rgrunber
rgrunber previously approved these changes Feb 23, 2023
Copy link
Member

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, so once the last comments are addressed, I thin this is good to squash + merge.

src/languageservice/parser/jsonParser07.ts Show resolved Hide resolved
src/languageservice/parser/jsonParser07.ts Outdated Show resolved Hide resolved
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@msivasubramaniaan msivasubramaniaan merged commit a11c112 into main Feb 23, 2023
@msivasubramaniaan msivasubramaniaan deleted the 832-format-ipv4-is-not-being-checked branch February 23, 2023 15:46
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.

Format IPv4 is not being checked
3 participants