Skip to content

Handle '/' in locale name in PAL#46601

Merged
tarekgh merged 1 commit intodotnet:masterfrom
am11:feature/globalization-native
Jan 6, 2021
Merged

Handle '/' in locale name in PAL#46601
tarekgh merged 1 commit intodotnet:masterfrom
am11:feature/globalization-native

Conversation

@am11
Copy link
Member

@am11 am11 commented Jan 5, 2021

Fixes #46577

@ghost
Copy link

ghost commented Jan 5, 2021

Tagging subscribers to this area: @tarekgh, @safern, @krwq
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #46577

Author: am11
Assignees: -
Labels:

area-System.Globalization

Milestone: -

Copy link
Member

@tarekgh tarekgh left a comment

Choose a reason for hiding this comment

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

Thanks @am11 for your help with this issue.

@tarekgh tarekgh changed the title Handle 0x2F locale name case in PAL Handle '/' locale name case in PAL Jan 5, 2021
@tarekgh tarekgh changed the title Handle '/' locale name case in PAL Handle '/' in locale name in PAL Jan 5, 2021
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

Thanks!

@tarekgh
Copy link
Member

tarekgh commented Jan 5, 2021

@am11 could you please hold on this? I am looking if we can have a better fix here.

@tarekgh
Copy link
Member

tarekgh commented Jan 5, 2021

@am11 I think we can go ahead with this fix here as it address the infinite looping and we can address the locale name validation in another PR. The reason here is we may decide later porting the infinite loop issue to 5.0. what you think?

@am11
Copy link
Member Author

am11 commented Jan 5, 2021

@tarekgh, I think it makes sense to keep the fix for infinite loop separate from exhaustive validation checks.

Is it so that ICU is validating some characters

[InlineData("en@US")]
[InlineData("\uFFFF")]
but not all that forbidden by BCP47 for locale names? or are these validations also implemented in the runtime code?

@tarekgh
Copy link
Member

tarekgh commented Jan 5, 2021

but not all that forbidden by BCP47 for locale names? or are these validations also implemented in the runtime code?

We manually handle the names containing @ in the runtime. But I don't think we manually validate the part preceding the @. For other cases like \uFFFF, I don't think this validation done in the runtime but should be done in ICU. we can easily confirm that.

@tarekgh tarekgh merged commit 81d58ab into dotnet:master Jan 6, 2021
@tarekgh
Copy link
Member

tarekgh commented Jan 6, 2021

@am11 thanks again for your help here. Are you interested to look at locale name validation when using ICU?

@am11
Copy link
Member Author

am11 commented Jan 6, 2021

@tarekgh, I can take a look at validation next.

@am11 am11 deleted the feature/globalization-native branch January 6, 2021 07:42
@ghost ghost locked as resolved and limited conversation to collaborators Feb 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The CultureInfo named "/" might stall the dotnet process on macOS/Linux

4 participants