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

Parser 2.0 does not validate channel name #740

Closed
ivangsa opened this issue Mar 30, 2023 · 10 comments
Closed

Parser 2.0 does not validate channel name #740

ivangsa opened this issue Mar 30, 2023 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@ivangsa
Copy link

ivangsa commented Mar 30, 2023

Describe the bug

Parser 2.0 has a bug and do not validate channel name

How to Reproduce

This was reported here
asyncapi/vs-asyncapi-preview#166

Expected behavior

It return a validation error like version 1.x:

6.TestChannel?Content-Type=application/json;charset=utf-8 channel contains invalid name with url query parameters: ?Content-Type=application/json;charset=utf-8

@ivangsa ivangsa added the bug Something isn't working label Mar 30, 2023
@github-actions
Copy link

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

Copy link
Member

derberg commented Mar 30, 2023

my guess is that https://github.com/asyncapi/parser-js/blob/master/lib/customValidators.js#L424 is not reflected in spectral rules for Parser 2 RC

@magicmatatjahu
Copy link
Member

ParserJS v2 validates it, but it's treated as warning, not error - when you put the mentioned AsyncAPI document you will see this:

image

Copy link
Member

derberg commented Mar 30, 2023

Yeah, but that is an error, spec says there SHALL NOT be query params. Parser 1 throws error, we should not have regression here. In case of custom validators from version 1 I remember that there was only once case where instead of error we should show warning

@magicmatatjahu
Copy link
Member

So it's only a changing the severity kind of given rule, in this place - https://github.com/asyncapi/parser-js/blob/next-major/src/ruleset/v2/ruleset.ts#L290

Copy link
Member

derberg commented Apr 3, 2023

@magicmatatjahu should it also be move under v2CoreRuleset ?

@magicmatatjahu
Copy link
Member

Can be moved.

Copy link
Member

derberg commented Apr 3, 2023

What needs to be done to solve:

Add severity: 'error' to https://github.com/asyncapi/parser-js/blob/next-major/src/ruleset/v2/ruleset.ts#L290 and move it under v2CoreRuleset

/gfi ts

@asyncapi-bot
Copy link
Contributor

Hey @derberg, your message doesn't follow the requirements, you can try /help.

@smoya
Copy link
Member

smoya commented Apr 13, 2023

A PR with the fix has been created #747

@volta-net volta-net bot closed this as completed Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants