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

Fix for not throwing and exception for an invalid enum value #1112

Merged
merged 1 commit into from Mar 16, 2018

Conversation

Projects
None yet
2 participants
@mikepizzo
Member

mikepizzo commented Mar 15, 2018

Issues

Description

When we added support for casting strings to enum values in URLs, we didn't validate that the string was a valid member of the enum type. This means that using a prefixed enum value would throw an exception if the value was not a member of the enum, but using the string form would not.

Checklist (Uncheck if it is not completed)

  • Build and test with one-click build and test script passed

Additional work necessary

string memberName = constantNode.Value.ToString();
IEdmEnumType enumType = targetTypeReference.Definition as IEdmEnumType;
if (enumType.Members.Any(m => m.Name == memberName))
{

This comment has been minimized.

@biaol-odata

biaol-odata Mar 15, 2018

Member

Should we use string.Equals here?

IEdmEnumType enumType = targetTypeReference.Definition as IEdmEnumType;
if (enumType.Members.Any(m => m.Name == memberName))
{
return new ConstantNode(new ODataEnumValue(constantNode.Value.ToString(), targetTypeReference.Definition.ToString()), constantNode.Value.ToString(), targetTypeReference);

This comment has been minimized.

@biaol-odata

biaol-odata Mar 15, 2018

Member

return [](start = 23, length = 7)

nit: put throw inside an "else" branch?

@biaol-odata

LGTM, looks like we are adding binding value check here upfront. We can check later WebAPI can pickup this change correctly.

@mikepizzo mikepizzo merged commit 3dc7ffe into OData:master Mar 16, 2018

2 checks passed

continuous-integration/vsts The build trigger has fired.
Details
license/cla All CLA requirements met.

@mikepizzo mikepizzo deleted the mikepizzo:ThrowExceptionForUndefinedEnumValue branch Mar 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment