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

Hotfix for the infinite loop if structured type has property which type is the declaring type #1855

Merged
merged 1 commit into from
Aug 17, 2020

Conversation

xuzhg
Copy link
Member

@xuzhg xuzhg commented Aug 13, 2020

Issues

This pull request fixes issue #xxx.

Description

The Url validation will go into Infinite loop if a structured type has the property which type is the declaring type.
Below is the call stack.

0000009d`e1986640 00007ffe`1300edc0 System_Core_ni!System.Collections.Generic.HashSet_1[[System.__Canon,_mscorlib]].SetCapacity+0x40
0000009d`e19866b0 00007ffe`1300eb72 System_Core_ni!System.Collections.Generic.HashSet_1[[System.__Canon,_mscorlib]].AddIfNotPresent+0x152
0000009d`e1986730 00007ffd`b90e99b0 Microsoft_OData_Core!Microsoft.OData.UriParser.Validation.ODataUrlValidator.ValidateTypeHierarchy+0xf0
0000009d`e19867c0 00007ffd`b90eb837 Microsoft_OData_Core!Microsoft.OData.UriParser.Validation.ODataUrlValidator.ValidateProperties+0x1a7
0000009d`e1986840 00007ffd`b90eb871 Microsoft_OData_Core!Microsoft.OData.UriParser.Validation.ODataUrlValidator.ValidateProperties+0x1e1
0000009d`e19868c0 00007ffd`b90eb871 Microsoft_OData_Core!Microsoft.OData.UriParser.Validation.ODataUrlValidator.ValidateProperties+0x1e1

This PR is the hot fix to fix it.

Checklist (Uncheck if it is not completed)

  • Test cases added
  • Build and test with one-click build and test script passed

Additional work necessary

If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.

@xuzhg xuzhg force-pushed the fixinfinitLoopInUriValidation branch from 579dc04 to ec243eb Compare August 13, 2020 22:41
Copy link
Contributor

@gathogojr gathogojr left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -413,14 +413,21 @@ private void ValidateImpliedProperties(IEdmType segmentType, SelectExpandClause
// Validate all structured properties of a type, recursing through nested complex typed properties
private void ValidateProperties(IEdmType edmType, ODataUrlValidationContext context)
{
IEdmStructuredType structuredType = edmType as IEdmStructuredType;
if (structuredType != null)
if (!context.ValidatedTypes.Contains(edmType))
Copy link
Member

Choose a reason for hiding this comment

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

(!context.ValidatedTypes.Contains(edmType) [](start = 15, length = 42)

ISet.Add returns false if the item was already in the list, so you can replace 416 with:

if (context.ValidatedTypes.Add(edmType))

and remove line 418, avoiding the extra look-up.

Copy link
Member Author

Choose a reason for hiding this comment

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

#Resolved

Copy link
Member

@mikepizzo mikepizzo left a comment

Choose a reason for hiding this comment

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

Minor optimization in checking validated types, otherwise looks good -- thanks!

@xuzhg xuzhg force-pushed the fixinfinitLoopInUriValidation branch from ec243eb to ad75d0c Compare August 17, 2020 20:26
@xuzhg xuzhg merged commit 941a11e into master Aug 17, 2020
@xuzhg xuzhg deleted the fixinfinitLoopInUriValidation branch August 17, 2020 20:27
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.

4 participants