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

RequestBuilder.BuildUri() returns broken Uri on Thai cultureinfo #2542

Closed
HJLeee opened this issue Sep 20, 2023 · 5 comments
Closed

RequestBuilder.BuildUri() returns broken Uri on Thai cultureinfo #2542

HJLeee opened this issue Sep 20, 2023 · 5 comments
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: question Request for information or clarification. Not an issue.

Comments

@HJLeee
Copy link

HJLeee commented Sep 20, 2023

Hello. One of my clients has reported a bug that occurs only in certain settings.

Environment details

  • Programming language: C#
  • OS: Linux
  • Language runtime version: .NET Core 3.1
  • Package version: 1.44 (and after)

Steps to reproduce

  1. Run this sample on .NET Core 3.1 + Linux env.
  class Program
  {
      static void Main(string[] args)
      {
          CultureInfo.CurrentCulture = new CultureInfo("th");
          foo();

          CultureInfo.CurrentCulture = new CultureInfo("en-US");
          foo();

      }

      static void foo()
      {
          var baseUriStr = "https://accounts.google.com/o/oauth2/v2/auth";
          var pathStr = "?access_type=offline~";

          var builder = new RequestBuilder()
          {
              BaseUri = new Uri(baseUriStr),
              Path = pathStr
          };

          Console.WriteLine($"{builder.BuildUri().AbsoluteUri}");
          Console.WriteLine($"{pathStr.StartsWith("/")} vs {pathStr.StartsWith("/", StringComparison.OrdinalIgnoreCase)}");
      }
  }
  1. Check the result.
https://accounts.google.com/?access_type=offline~
True vs False
https://accounts.google.com/o/oauth2/v2/auth?access_type=offline~
False vs False

This is due to a bug in .NET Core 3.1 inappropriately handling Thai language and luckly it is fixed in .NET 5.
As the sample code shows, it needs to add StringComparison.OrdinalIgnoreCase option to string.StartsWith() to work around the bug.

I found that .NET runtime code also uses StringComparison.OrdinalIgnoreCase internally for Uri handling.
https://github.com/dotnet/runtime/blob/9cbad65fe68bc518fee67f7d75056ea0651e6221/src/libraries/System.Private.Uri/src/System/Uri.cs#L1915

Thank you.

@HJLeee HJLeee added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Sep 20, 2023
@amanda-tarafa amanda-tarafa added type: question Request for information or clarification. Not an issue. and removed type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Sep 21, 2023
@amanda-tarafa
Copy link
Contributor

We are discussing this internally. For now I've marked this issue as a question as there's really not a bug in libraries. If anything we'll be applying a workaround for a bug in .NET Core 3.1.

Do you have a reference to an issue in .NET Core 3.1 that we can look at? Our main worry is that if we work around the forward slash comparison, what else can we find down the road that is also not properly handled with Thai culture by .NET?

@amanda-tarafa amanda-tarafa self-assigned this Sep 21, 2023
@HJLeee
Copy link
Author

HJLeee commented Sep 21, 2023

Do you have a reference to an issue in .NET Core 3.1 that we can look at?

Sure. The below link shows there are behavior differences among .NET versions and how to deal with it from .NET developer perspective.

My original post concerns only my circumstances which is .NET 3.1 + Linux with string.StartsWith("/") on Thai culture but you can find other related issues posted on dotnet/runtime repo.
They are not limited to .NET 3.1 but also occurs on other .NET versions.

Also there is an internal discussion about it since 2020 but not yet concluded.

So, some of .NET components like MAUI landed PR like this.

jskeet added a commit to jskeet/google-api-dotnet-client that referenced this issue Sep 22, 2023
jskeet added a commit to jskeet/google-api-dotnet-client that referenced this issue Sep 22, 2023
@amanda-tarafa
Copy link
Contributor

Thanks @HJLeee . See #2542 who should work around your issue.

@amanda-tarafa
Copy link
Contributor

@HJLeee we have released v1.62.1 of the support libraries: Google.Apis.Core, Google.Apis, Google.Apis.Auth, Google.Apis.Auth.AspNetCore3 with the workaround in #2542.

Please make certain you add explicit references to these packages.

I'll be closing this issue now as I believe there's nothing else we can do here, but do leave a comment if you believe otherwise.

@HJLeee
Copy link
Author

HJLeee commented Sep 25, 2023

#2546 looks good. I appreciate for your fast PR landing and release. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

No branches or pull requests

3 participants