Skip to content

Ask About HttpRequest.PhysicalPath #373

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

Closed
Clounea opened this issue Jul 13, 2023 · 4 comments · Fixed by #410
Closed

Ask About HttpRequest.PhysicalPath #373

Clounea opened this issue Jul 13, 2023 · 4 comments · Fixed by #410
Labels
enhancement New feature or request In-PR Issues that have a PR open for them.

Comments

@Clounea
Copy link
Contributor

Clounea commented Jul 13, 2023

Summary

Our repo uses HttpRequest.PhysicalPath which is not supported by ASP.NetCore and this adapters.
I want to ask if there is equivalent and if there is plan to add them to adapter.

Motivation and goals

We need migrate code (an API in Sharepoint) to ASP.NetCore and we find that the APIs are not supported by ASP.NetCore.

In scope

HttpRequest.PhysicalPath

From src code, it uses HostingEnvironment.MapPathInternal which is related to this issue: Ask About System.Web.Hosting.HostingEnvironment

Examples

 if (null != context.Request
                 && null != context.Request.PhysicalPath
                 && IsSuspiciousPhysicalPath(context.Request.PhysicalPath))
{
    Log("PhysicalPath is suspicious");
}
@dotnet-policy-service dotnet-policy-service bot added the Needs: Triage 🔍 Label added to new issues which need Triage label Jul 13, 2023
@twsouthwick
Copy link
Member

This is a tricky one as there is really no concept of physical paths in ASP.NET Core. It could probably be added to IHttpRequestPathFeature and surfaced that way.

@twsouthwick
Copy link
Member

This may be able to be implemented by:

internal interface IHttpRequestPathFeature
{
+  string? PhysicalPath { get; }
}

With a default implementation that would look at IWebHostEnvironment.WebRootFileProvider and see if it is a physical path.

OR the default implementation could just be null (looks like you check for null) since they don't really point to a physical path anymore

@CZEMacLeod - I know you've looked into mapping the various IFileProvider stuff into this kind of thing, so checking if you have thoughts.

@CZEMacLeod
Copy link
Contributor

@twsouthwick Interesting. Most of the stuff I was looking at was related to Path, PathInfo and FilePath - although as you say, most requests (other than those that would be consumed by the StaticFile middleware) won't have an actual physical file.
In Asp.Net terms, it runs as if you were using the runAllManagedModulesForAllRequests flag, so you don't need an actual file to trigger mapping into dotnet.
It also relates somewhat to the work you and I both had a look at to do with IHttpHandler.
I like the idea of checking for a physical file using the FileProvider. I have some code for wrapping the existing FileProvider as a VirtualPathProvider so it can register as the default MapPathVirtualPathProvider equivalent, then wrapping the VPP chain as a FileProvider for the StaticFiles middleware. This would work well with that to allow VirtualFiles to also be handled correctly.
I feel like the default provider is perhaps a null provider, but you could configure it to use the more advanced, but perhaps slower, one with IFileProvider, perhaps taken as an input to a WithPhysicalPathProvider(this IServiceCollection services, IFileProvider provider) type extension method to replace the default one registered?
It might be worthwhile noting that one thing that is less clear in Asp.Net is the separation of client content, from the application resources. This means that it is difficult to know whether to use WebRootFileProvider or ContentRootFileProvider at times. I haven't had time to check if there is much work already done on how things like App_Data and other normally blocked paths are handled in the Core side?

@joperezr joperezr added enhancement New feature or request and removed Needs: Triage 🔍 Label added to new issues which need Triage labels Aug 30, 2023
twsouthwick added a commit that referenced this issue Oct 4, 2023
This change just adds the API with a default to null as we don't really have an equivalent for what a physical path is in ASP.NET Core.

Fixes #373
@dotnet-policy-service dotnet-policy-service bot added the In-PR Issues that have a PR open for them. label Oct 4, 2023
@twsouthwick
Copy link
Member

I'm putting a change up to surface the API. If we enable #332 by exposing the internal features, then someone could build on top of it to enable some semantics that make sense here

twsouthwick added a commit that referenced this issue Oct 4, 2023
This change just adds the API with a default to null as we don't really have an equivalent for what a physical path is in ASP.NET Core.

Fixes #373
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request In-PR Issues that have a PR open for them.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants