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

EnsureValidDataType should reject whitespaces in DataTypeAttribute.CustomDataType property #15690

Closed
Tracked by #93172 ...
sparraguerra opened this issue Nov 12, 2015 · 4 comments
Assignees
Labels
area-System.ComponentModel.DataAnnotations bug disabled-test The test is disabled in source code against the issue help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@sparraguerra
Copy link
Contributor

Method EnsureValidDataType() should not raise an exception when the property CustomDataType is a WhiteSpace?

This code:

/// <summary>
///  Throws an exception if this attribute is not correctly formed
/// </summary>
/// <exception cref="InvalidOperationException"> is thrown if the current attribute is ill-formed.</exception>
private void EnsureValidDataType()
{
    if (DataType == DataType.Custom && string.IsNullOrEmpty(CustomDataType))
    {
         throw new InvalidOperationException(string.Format(CultureInfo.CurrentCulture,
               SR.DataTypeAttribute_EmptyDataTypeString));
     }
}

Should be?

/// <summary>
///  Throws an exception if this attribute is not correctly formed
/// </summary>
/// <exception cref="InvalidOperationException"> is thrown if the current attribute is ill-formed.</exception>
private void EnsureValidDataType()
{
    if (DataType == DataType.Custom && string.IsNullOrWhiteSpace(CustomDataType))
    {
         throw new InvalidOperationException(string.Format(CultureInfo.CurrentCulture,
               SR.DataTypeAttribute_EmptyDataTypeString));
     }
}
@sparraguerra sparraguerra changed the title Modify EnsureValidDataType() method in src/System.ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/DataTypeAttribute.cs Modify EnsureValidDataType() method in System.ComponentModel.DataAnnotations.DataTypeAttribute Nov 12, 2015
@zhenlan zhenlan assigned lajones and unassigned zhenlan Nov 13, 2015
@karelz
Copy link
Member

karelz commented Nov 14, 2016

Sounds reasonable, can you submit a PR?

BTW: Sorry for the overdue answer :( ... we are catching up now.

@karelz karelz changed the title Modify EnsureValidDataType() method in System.ComponentModel.DataAnnotations.DataTypeAttribute EnsureValidDataType should reject whitespaces in DataTypeAttribute.CustomDataType property Nov 14, 2016
@divega
Copy link
Contributor

divega commented Jan 4, 2017

@karelz was this fixed?

@karelz
Copy link
Member

karelz commented Jan 4, 2017

Not yet - the PR was against wrong branch (future) - see dotnet/corefx#14654 for more details
@sparraguerra can you please submit your changes against master branch? Very sorry for the inconvenience ... I should have caught the wrong branch earlier :(

karelz referenced this issue in dotnet/corefx Jan 12, 2017
…stomDataType property because it is mandatory and it does not make sense that the property is whitespaces. (#14845)

Modify Ctor_String(string customDataType) test to validate whitespaces adding new InlineData.

Fix #4465
@ajcvickers
Copy link
Member

ajcvickers commented Apr 13, 2017

Marking this for triage since there is now a behavior difference between .NET Core and .NET Framework and we need to re-evaluate if this is okay given our current understanding.

See dotnet/corefx#17874

@ajcvickers ajcvickers reopened this Apr 13, 2017
@lajones lajones closed this as completed Apr 18, 2017
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.ComponentModel.DataAnnotations bug disabled-test The test is disabled in source code against the issue help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

No branches or pull requests

7 participants