aspnet / FileSystem

[Archived] Abstraction of file system APIs. Project moved to https://github.com/aspnet/Extensions
Apache License 2.0
171 stars 82 forks source link

use StartWith with StringComparison.Ordinal to properly detect hidden files on Linux #311

Closed sherlock1982 closed 6 years ago

sherlock1982 commented 6 years ago

When detecting if the file is hidden or not we need to use StartWith together with StringComparison.Ordinal otherwise in some cultures all files will be hidden.

I see this issue is already fixed in dev branch but I suggest to fix it in the next release as well.

natemcmaster commented 6 years ago

Hey @sherlock1982, just wanted to let you know we've seen the PR. Generally, there is a high bar for putting changes into a patch: high-impact bugs, security issues, etc. Are there any other details you can share about how this bug is affecting you? Have you been able to workaround it? Is it causing hidden files to become visible?

in some cultures all files will be hidden.

Can you provide a repro that shows this issue? It would be ideal to add it as a test so we don't regress in the future.

cc @muratg @Eilon

sherlock1982 commented 6 years ago

I run my application in different locales. I noticed that in th_TH.UTF-8 on Linux Kestrel returns 404 for any static file. After some debugging I found out that the root cause of it is that StartsWith('.') for some reason returns true for filenames while running in this locale.

I was able to make a workaround for this issue. I implemented my own PhysicalFileProvider (well mostly copypasted existing one) and put it eveywhere: replaced in HostingEnvironment, put inside options for StaticFiles middleware.

I thought it might be helpful for others as well. This issue stopped us installing software internationally. Also I noticed that in dev branch it's already like this.

Eilon commented 6 years ago

@Petermarcu - is there someone on your team who is a locale/culture expert on strings? This behavior seems odd. I do agree the ASP.NET Core code should have explicitly passed in Ordinal for the comparison type, but I'm surprised there's a culture where this doesn't work as expected...

natemcmaster commented 6 years ago

Fun. Repros on macOS too.

        static void Main(string[] args)
        {
            CultureInfo.CurrentCulture = new CultureInfo("en_US.UTF-8");
            CultureInfo.CurrentUICulture = new CultureInfo("en_US.UTF-8");
            Console.WriteLine("This".StartsWith(".")); // False

            CultureInfo.CurrentCulture = new CultureInfo("th_TH.UTF-8");
            CultureInfo.CurrentUICulture = new CultureInfo("th_TH.UTF-8");
            Console.WriteLine("This".StartsWith(".")); // True
        }
Eilon commented 6 years ago

Let's take this offline and deal with it via email.

aspnet-hello commented 6 years ago

This issue was moved to aspnet/Home#2530

aspnet-hello commented 6 years ago

Sorry I was a bad bot. Re-opening this PR.

Eilon commented 6 years ago

@natemcmaster was this closed by mistake? Due to branch changes?

natemcmaster commented 6 years ago

Yup, was closed by GitHub due to branch rename from release/2.0.0 to release/2.0. I've retargeted to the new base branch.

@sherlock1982 sorry for the noise on this. I need to make a couple changes to release/2.0 first (like versioning bumps), but I'm planning to merge this in later today.

natemcmaster commented 6 years ago

We have a candidate fix available on a nightly feed. If you would like to try it out, follow these steps:

  1. Add a NuGet.config file to your solution and with this patch feed:
<configuration>
  <packageSources>
    <add key="aspnetcore-test-feed" value="https://dotnet.myget.org/F/aspnet-2018-feb-patch-public/api/v3/index.json" />
  </packageSources>
</configuration>
  1. Update your MSBuild project (*.csproj file) to include a reference to the patched component, Microsoft.Extensions.FileProviders.Physical

    <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.FileProviders.Physical" Version="2.0.1-rtm-215" />
    </ItemGroup>
  2. Dotnet restore and re-build your project.

Please let me know ASAP if this does not fix the issue or causes a regression of any kind. We're preparing to release this fix to NuGet.org soon.