Skip to content

Bypass CreateReadStream for PhysicalFileInfo in FileConfigurationProvider - #131486

Draft
rosebyte wants to merge 1 commit into
dotnet:mainfrom
rosebyte:rosebyte-expert-fishstick
Draft

Bypass CreateReadStream for PhysicalFileInfo in FileConfigurationProvider#131486
rosebyte wants to merge 1 commit into
dotnet:mainfrom
rosebyte:rosebyte-expert-fishstick

Conversation

@rosebyte

Copy link
Copy Markdown
Member

Fixes #47455

Since .NET 5, a custom IFileProvider whose IFileInfo reports a PhysicalPath but transforms the content in CreateReadStream is silently ignored by the configuration system. The classic case is an encrypted appsettings.json that a provider decrypts on read: AddJsonFile now hands the parser the raw encrypted bytes straight off the disk rather than the decrypted stream.

PhysicalPath is documented as "the path to the file, including the file name, or null if the file is not directly accessible". It says nothing about the bytes at that path being the bytes CreateReadStream would produce. That equivalence is an implementation detail of PhysicalFileInfo, not part of the IFileInfo contract, so relying on it breaks any provider that layers on top of a real file.

Root cause

#37846 added a fast path to FileConfigurationProvider.Load that opens a FileStream with async I/O disabled, since the configuration system reads synchronously and the FileOptions.Asynchronous that PhysicalFileInfo.CreateReadStream requests is pure overhead here. The optimisation is worth having; the trouble is only that it was gated on fileInfo.PhysicalPath != null, which is true for far more implementations than the one it was written for.

Why the check is on the file info rather than the file provider

The earlier attempt at this, #47934 (closed for inactivity, not on merit), checked source.FileProvider.GetType() == typeof(PhysicalFileProvider) instead. Checking the file info is strictly better:

  • It never gives up a fast path the provider check would have kept. PhysicalFileProvider.GetFileInfo returns only NotFoundFileInfo or PhysicalFileInfo, and NotFoundFileInfo is already filtered out by the !file.Exists branch above. So wherever the provider check would fire, the file info check fires too.
  • It keeps the fast path for delegating providers. CompositeFileProvider and anything else that surfaces an inner PhysicalFileProvider's PhysicalFileInfo unchanged would have been quietly deoptimised by the provider check. CompositeFileProvider is common enough in ASP.NET Core hosting that this matters.
  • There is nothing to cache and nothing to go stale. FileConfigurationSource.FileProvider is a mutable public property read afresh on every Load, so a provider-type flag computed in the constructor (as suggested in review on the old PR) could be wrong by the time it is used. A per-call check on the file info has no such asymmetry.

PhysicalFileInfo.PhysicalPath returns FileInfo.FullName, which is never null, so the null check is not merely redundant after the change but provably unnecessary.

No new dependency is introduced: Microsoft.Extensions.FileProviders.Physical is already an unconditional ProjectReference of this project, used by FileConfigurationSource.ResolveFileProvider. There is no trim or AOT concern either, as this is type identity only and no reflection.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR narrows the FileConfigurationProvider “physical path” fast path so it only bypasses IFileInfo.CreateReadStream() when the IFileInfo is exactly PhysicalFileInfo, preserving the sync-IO optimization for the built-in physical provider while restoring correct behavior for custom/delegating providers that transform file contents in CreateReadStream().

Changes:

  • Update FileConfigurationProvider to use the sync FileStream fast path only when fileInfo.GetType() == typeof(PhysicalFileInfo).
  • Add coverage to ensure providers that return a PhysicalPath but transform content via CreateReadStream() are not bypassed.
  • Adjust the existing FileInfoImpl.CreateReadStream() test helper to open the physical file so existing exception-path tests remain meaningful with the new gating.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs Restricts the fast-path FileStream open to exact PhysicalFileInfo instances instead of any non-null PhysicalPath.
src/libraries/Microsoft.Extensions.Configuration.FileExtensions/tests/FileConfigurationProviderTest.cs Adds a new [Theory] covering physical/delegating/derived/custom providers and updates a test IFileInfo helper to open the actual file stream.

Comment on lines +198 to +199
Assert.Equal(expectedContent, provider.Content);
Assert.Equal(expectedSynchronousRead, provider.ReadSynchronously);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Json Configuration custom IFileProvider stopped working for .NET 5.0

2 participants