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

Utf8Parser.TryParse ignores the provided format for bool #100659

Closed
joegoldman2 opened this issue Apr 5, 2024 · 2 comments · Fixed by #103861
Closed

Utf8Parser.TryParse ignores the provided format for bool #100659

joegoldman2 opened this issue Apr 5, 2024 · 2 comments · Fixed by #103861
Assignees
Labels
area-System.Memory documentation Documentation bug or enhancement, does not impact product or test code in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@joegoldman2
Copy link
Contributor

joegoldman2 commented Apr 5, 2024

Description

The current implementation of Utf8Parser.TryParse for bool ignores the format:

public static bool TryParse(ReadOnlySpan<byte> source, out bool value, out int bytesConsumed, char standardFormat = default)
{
if (!(standardFormat == default(char) || standardFormat == 'G' || standardFormat == 'l'))
ThrowHelper.ThrowFormatException_BadFormatSpecifier();
if (source.Length >= 4)
{
int dw = BinaryPrimitives.ReadInt32LittleEndian(source) & ~0x20202020;
if (dw == 0x45555254 /* 'EURT' */)
{
bytesConsumed = 4;
value = true;
return true;
}
if (source.Length > 4)
{
if (dw == 0x534c4146 /* 'SLAF' */ && (source[4] & ~0x20) == 'E')
{
bytesConsumed = 5;
value = false;
return true;
}
}
}
bytesConsumed = 0;
value = default;
return false;
}

Reproduction Steps

byte[] bytes = Encoding.UTF8.GetBytes("true");
var result = Utf8Parser.TryParse(bytes, out bool value, out int byteConsumed, standardFormat: 'G');
Console.WriteLine(result);

Expected behavior

result should be false because for the format G, True or False beginning with uppercase are expected.

Actual behavior

result is true.

Regression?

I don't think so. In dotnet/corefx, the implementation already didn't take the format into account.

Known Workarounds

No response

Configuration

9.0.0-preview.2.24128.5

Other information

No response

Copy link
Contributor

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

@jeffhandley
Copy link
Member

Note that we decided against making the behavioral change per the comments in #100676, so a docs change is being pursued instead in #103861.

Thanks for submitting this issue and driving the changes, @joegoldman2!

@jeffhandley jeffhandley added documentation Documentation bug or enhancement, does not impact product or test code and removed untriaged New issue has not been triaged by the area owner labels Jul 19, 2024
@jeffhandley jeffhandley added this to the 9.0.0 milestone Jul 19, 2024
@jeffhandley jeffhandley assigned buyaa-n and krwq and unassigned buyaa-n Aug 12, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Aug 17, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Memory documentation Documentation bug or enhancement, does not impact product or test code in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
4 participants