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

Validating ABNF with an ABNF parser fails for some ODATA URLs #99

Open
thewahome opened this issue Jun 19, 2023 · 8 comments
Open

Validating ABNF with an ABNF parser fails for some ODATA URLs #99

thewahome opened this issue Jun 19, 2023 · 8 comments

Comments

@thewahome
Copy link

On Graph Explorer, we use these construction rules together with an ABNF parser to validate URLs entered by our users have the correct syntax.

We have some URLs being marked as invalid even though they are. For instance,

https://graph.microsoft.com/v1.0/sites/m365x55726300.sharepoint.com,f2102bda-0e14-4fc5-bbe1-e65a13cd91a4,6be9dd5c-0640-4465-b955-1528279a0227/lists/7ec9d64f-732f-483b-b1d0-5abb149869f1?$expand=drive,items

We also have some that only pass when a trailing slash is added:
https://github.com/microsoftgraph/microsoft-graph-explorer-v4/blob/898e7dc7d78d59a173e0783c2a60a9385d40d5c6/src/modules/validation/abnf.spec.ts#L32

@HeikoTheissen
Copy link
Contributor

The apg-js parser is too greedy and gobbles up the resourcePath as part of the serviceRoot.

@ralfhandl
Copy link
Contributor

AFAIK that can't be avoided for key-as-segment URLs.

@thewahome does the parser accept the "traditional" URL

https://graph.microsoft.com/v1.0/sites('m365x55726300.sharepoint.com,f2102bda-0e14-4fc5-bbe1-e65a13cd91a4,6be9dd5c-0640-4465-b955-1528279a0227')/lists('7ec9d64f-732f-483b-b1d0-5abb149869f1')?$expand=drive,items

@HeikoTheissen
Copy link
Contributor

https://graph.microsoft.com/v1.0/sites('m365x55726300.sharepoint.com,f2102bda-0e14-4fc5-bbe1-e65a13cd91a4,6be9dd5c-0640-4465-b955-1528279a0227')/lists('7ec9d64f-732f-483b-b1d0-5abb149869f1')?$expand=drive,items

The apg-js parser is still too greedy:

Issue 99 fails at 190: https://graph.microsoft.com/v1.0/sites('m365x55726300.sharepoint.com,f2102bda-0e14-4fc5-bbe1-e65a13cd91a4,6be9dd5c-0640-4465-b955-1528279a0227')/lists('7ec9d64f-732f-483b-b1d0-5abb149869f1')?$expand=drive,items
odataUri: https://graph.microsoft.com/v1.0/sites('m365x55726300.sharepoint.com,f2102bda-0e14-4fc5-bbe1-e65a13cd91a4,6be9dd5c-0640-4465-b955-1528279a0227')/lists
.serviceRoot: https://graph.microsoft.com/v1.0/sites('m365x55726300.sharepoint.com,f2102bda-0e14-4fc5-bbe1-e65a13cd91a4,6be9dd5c-0640-4465-b955-1528279a0227')/
..host: graph.microsoft.com
...reg-name: graph.microsoft.com
..segment-nz: v1.0
..segment-nz: sites('m365x55726300.sharepoint.com,f2102bda-0e14-4fc5-bbe1-e65a13cd91a4,6be9dd5c-0640-4465-b955-1528279a0227')
..segment-nz: lists('7ec9d64f-732f-483b-b1d0-5abb149869f1')
.odataRelativeUri: lists
...

@ralfhandl
Copy link
Contributor

Maybe we should reduce the ABNF to URLs relative to the service root.

@thewahome
Copy link
Author

The apg-js parser is still too greedy

@HeikoTheissen should I use a different parser?

@HeikoTheissen
Copy link
Contributor

The apg-js parser is still too greedy

@HeikoTheissen should I use a different parser?

No sure whether the problem is with the parser or with the ABNF rules. (Sounds like theoretical computer science, not my area of expertise.) @ralfhandl, can you answer this?

@ralfhandl
Copy link
Contributor

The ABNF rules in this repository are documentation intended for human readers. They are not intended for generating a ready-to-use URL parser.

Many rule alternatives require the reader to know the underlying entity data model. This is especially true for URLs using key-as-segment notation: /foo/bar/baz could address

  • the entity in entity set foo with two-part key and the key values bar and baz
  • the property baz of an entity in set foo with single-part key and the key value bar
  • the property baz of an entity related to singleton foo via the single-valued navigation property bar
  • lots of other stuff

For client-side validation of URLs you'd need either a two-step approach where a context-unaware parser chunks up the URL into "segment: foo, segment: bar, segment: baz", and a model-aware validator that decides what each segment means. Or a model-aware parser that starts parsing left and then chooses the next rule alternatives based on what it has recognized so far and matched against the model.

There's an open source project for building such a parser, see https://github.com/oasis-open/odata-rapid/tree/main/tools/odataUri, with contributors mainly from Microsoft. Maybe you can join that effort and use it for the Graph Explorer.

@ralfhandl
Copy link
Contributor

ralfhandl commented May 27, 2024

See also oasis-tcs/odata-specs#377

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

3 participants