Skip to content

Add HttpPostedFile*. #260

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

Merged
merged 5 commits into from
Feb 17, 2023
Merged

Add HttpPostedFile*. #260

merged 5 commits into from
Feb 17, 2023

Conversation

johnLwith
Copy link
Contributor

@johnLwith johnLwith commented Dec 28, 2022

Add missing property of File in HttpRequest.

Fixes #141

Copy link
Member

@twsouthwick twsouthwick left a comment

Choose a reason for hiding this comment

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

LGTM

public IList<HttpPostedFile> GetMultiple(string name) => FormFiles.GetFiles(name) is { Count: > 0 } files ? new ReadOnlyPostedFileCollection(files) : Array.Empty<HttpPostedFile>();

public HttpPostedFile? this[string name] => Get(name);

public HttpPostedFile? this[int index] => Get(index);
Copy link
Member

Choose a reason for hiding this comment

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

@Tratcher I think this is the first of the namevaluecollection types we can actually fully conform - I hadn't realized IFormFileCollection had a this[int index] parameter

Copy link
Member

Choose a reason for hiding this comment

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

Ha, yeah, it's a full IReadOnlyList.


namespace System.Web;

public class HttpPostedFileWrapper : HttpPostedFileBase
Copy link
Member

Choose a reason for hiding this comment

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

Are we making all of the wrapper types public? Why?

Copy link
Member

Choose a reason for hiding this comment

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

This is a public API from .NET Framework: https://learn.microsoft.com/en-us/dotnet/api/system.web.httppostedfilewrapper?view=netframework-4.8.1

It's a convention used for a number of types. Most common is HttpContext which has no virtual methods. HttpContextBase was an abstraction for it. HttpContextWrapper wrapped HttpContext to HttpContextBase.

@twsouthwick
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@twsouthwick
Copy link
Member

@johnLwith this is approved, but we've updated to start using .NET 8 SDK, and it enabled new warnings. Can you address them and then we'll merge it in

@twsouthwick
Copy link
Member

@johnLwith do you think you'll be able to address the errors in the build?


public abstract class HttpPostedFileBase
{
public virtual string FileName => throw new NotImplementedException();
Copy link
Member

Choose a reason for hiding this comment

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

@johnLwith for these kind of warnings/errors, suppress with an attribute, and use the justification of Constants.ApiFromAspNet

@twsouthwick
Copy link
Member

@johnLwith will you be able to address the build error?

@twsouthwick
Copy link
Member

@johnLwith I went ahead and pushed a fix to your branch with the missing attributes

@twsouthwick twsouthwick merged commit ee231f4 into dotnet:main Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add HttpRequestBase.Files and supporting types
3 participants