SizeConverter is trying to convert even before verifying the value could be converted

[db410009]

Description

Two issues here:

CanConvertFrom returns true when the input parameter is a string.

This is not correct:

• The string should contain exactly two elements separated by the "culture" list separator
• It should be possible than both elements are converted into int numbers

ConvertFrom is trying to convert the values even the list of values provided has not 2 elements

This does not look "friendly".
If the result of the split is not returning exactly two elements, what is the sense of trying to convert them?. Even if you are able to convert, the result will be a exception raised because the number of elements

In case there are problems with the list separator (different culture when the value was created than when it is consumed), it will be much more useful raise the exception before trying to convert the elements with a message describing the problem:
"Unable to get the expected elements (2) when splitting the string ("my string") using the list separator ("sep") from the regional settings (culture)"

Reproduction Steps

Try to convert "1024,768" when regional settings is es_ES

On es_ES, the list separator is semi-colon. So, "1024,768".split(";") returns "1024,768" and then the try to convert to int crashes but the message is confuse, because 1024,768 is a valid number on Spanish.

Expected behavior

Try to convert "1024,768" when regional settings are es_ES throws an exception with a message like:
"Unable to create a Size from '1024,768' string because it was not possible to extract the required two components using the separator ';' from culture 'es_ES'"

Actual behavior

Try to convert "1024,768" when regional settings are es_ES throws an exception because 1024,768 is not a valid integer

Regression?

No response

Known Workarounds

On Windows modify the regional settings to use "," as list separator instead ";"

Configuration

No response

Other information

No response

[ghost]

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

Two issues here:

CanConvertFrom returns true when the input parameter is a string.

This is not correct:

• The string should contain exactly two elements separated by the "culture" list separator
• It should be possible than both elements are converted into int numbers

ConvertFrom is trying to convert the values even the list of values provided has not 2 elements

This does not look "friendly".
If the result of the split is not returning exactly two elements, what is the sense of trying to convert them?. Even if you are able to convert, the result will be a exception raised because the number of elements

In case there are problems with the list separator (different culture when the value was created than when it is consumed), it will be much more useful raise the exception before trying to convert the elements with a message describing the problem:
"Unable to get the expected elements (2) when splitting the string ("my string") using the list separator ("sep") from the regional settings (culture)"

Reproduction Steps

Try to convert "1024,768" when regional settings are es_ES

On es_ES, the list separator is semi-colon. So, "1024,768".split(";") returns "1024,768" and then the try to convert to int crashes but the message is confuse, because 1024,768 is a valid number on Spanish.

Expected behavior

Try to convert "1024,768" when regional settings are es_ES throws an exception with a message like:
"Unable to create a Size from '1024,768' string because it was not possible to extract the required two components using the separator ';' from culture 'es_ES'"

Actual behavior

Try to convert "1024,768" when regional settings are es_ES throws an exception because 1024,768 is not a valid integer

Regression?

No response

Known Workarounds

On Windows modify the regional settings to use "," as list separator instead ";"

Configuration

No response

Other information

No response

Author:	db410009
Assignees:	-
Labels:	area-System.ComponentModel
Milestone:	-

[ericstj]

CanConvertFrom returns true when the input parameter is a string.

This is not correct:

The string should contain exactly two elements separated by the "culture" list separator
It should be possible than both elements are converted into int numbers

CanConvertFrom is not validating the content of a string. It's only considering the type. This is all it can do, because it does not receive a value: https://learn.microsoft.com/en-us/dotnet/api/system.componentmodel.typeconverter.canconvertfrom?view=net-7.0

Changing the order of validation here would be breaking, but could be done to give a better error in this case.

runtime/src/libraries/System.ComponentModel.TypeConverter/src/System/Drawing/SizeConverter.cs

Lines 48 to 51 in 84a8f67

	if (values.Length != 2)
	{
	throw new ArgumentException(SR.Format(SR.TextParseFailedFormat, text, "Width,Height"));
	}

Regardless of what we do, we should probably fix that string, since it's explicitly using a , when the code above it uses the culture's actual list separator.

[steveharter]

Regardless of what we do, we should probably fix that string, since it's explicitly using a , when the code above it uses the culture's actual list separator.

So we would want something like

throw new ArgumentException(SR.Format(SR.TextParseFailedFormat, text, $"Width{sep}Height")); 

[fdahlen]

I'd like to try fixing the string separator part in the Exception message, if this would help?

However - it seems like the same problem exists also for PointConverter, RectangleConverter and SizeFConverter in the same namespace as SizeConverter.

Should this be filed as a new separate issue, or done within #92424?

Advice appreciated, since this would be my first time contributing.