File uploads to the server

[guardrex]

Fixes #20323

Internal Review Topic (links to section)

🍖

I'm embracing the same goals for the sample code here that we used when generating the coverage for MVC/RP file uploads in Upload files in ASP.NET Core. Example code only at this point. If we decide to proceed with this sample code and after we firm it up, then I'll work on the text.

On the PR are some 🙈 RexHack'ins™ 🙈 ... prototyped from your, @EdCharbeneau, code along the goals that I lay out below 👇. Coverage of the new section starts on Line 90 of the PR.

@pranavkm, do you want to supply feedback on this, or shall I close this PR in favor of a sample that you provide? If you decide to build the sample, you can link a GH repo in the issue, and I'll generate a new PR from there.

The two high-level goals are ...

• Show something useful that can be the basis for further customization and that deals with typical challenges.
• Show something that's 👮‍♀️ SAFE 👮‍♂️ to start with that both covers some basic security concepts and gets the reader thinking about additional security steps for their scenario. Where appropriate, we'll cross-link the security guidance in the File uploads topic.

The specific scenarios are ...

• Build on the existing client-side example in the topic that merely shows thumbnails. For the server upload example, let's continue to show thumbnails with the new code client-side and flesh out server-side upload.
• Provide a multiple file uploader for this, which is a bit more challenging to work out.
• I recommend that we DO and not DO some things with the sample code ...
  • Let's not show an example of accessing untrusted/unscanned files. For example, files shouldn't be directly uploaded to webroot 😨 or made instantly available or used in any other way.
  • Let's not trust the provided file names for storage or display 😨.
  • Let's show how how to limit the number of files client- and server-side.
  • Let's show how to limit the size of files client- and server-side.
  • Let's do some server-side logging with error details and show some client-side generic errors.
  • Let's not go so far as dealing with the following, and I'll add text later to explain some of the ways in which the example can be further developed:
    • Overwriting for duplicate uploads, but let's do prevent duplicate uploads client-side.
    • Saving files by-user, but I'll call it out. I'll mention that a folder-per-user is a common approach.
    • Limiting file uploads by image dimensions on the server, but let's do a resize on the client.
    • File extension and file signature checking, but let's cross-link to those sections in the File uploads topic for more info. The code is the same.

[guardrex]

For Blazor Server until the framework provides goodness at 6.0, what's the correct approach today? ... JS interop downsizing? (sending, too?) ... or just warn readers off of large file sizes (and what is 'large' in these contexts)?

Should we pivot the topic between WASM and Server and leave the examples in the topic and on the PR for the WASM pivot (with feedback updates)? If so ... is it ok for WASM examples to use downsized image data at 640x480 max dimensions and maxFileSize reduced from the current 15 MB down to 1 MB? I can warn about file sizes in the text after we get the examples right.

[guardrex]

@pranavkm @javiercn Latest bits on the PR now.

❓ ... Can we leave the base 64 string and thumbnail just for WASM? I'll set up the topic as a WASM-Server pivot, and only the WASM pivot will show it. I'll yank the thumbnail bits (and base 64 string) from the Server pivot.

Latest round of hacks 🙈 are on the PR. Hopefully, you won't grind your 🦷 too much looking at it.

The approach now is to use StreamContent ... IF I did it right. It seems to run at least ...

[javiercn]

❓ ... Can we leave the base 64 string and thumbnail just for WASM? I'll set up the topic as a WASM-Server pivot, and only the WASM pivot will show it. I'll yank the thumbnail bits (and base 64 string) from the Server pivot.

I would prefer if we don't. It just teaches folks bad practices. You still end up with a huge allocation in wasm and that'll have a negative effect in performance too.

[guardrex]

Fair enough ... it simplifies what I need to do here. We don't need a pivot.

I'll strip it out on the next commit. I'll do that right now and ping back in a few minutes.

[guardrex]

Done ... but I still need to go up to the first example and yank that stuff there. I'll update the first example later, tomorrow morning probably.

Here's what we have now ... just a list of files.

One thing that I need fix. FileUpload returns false briefly (apparently), so the error messages flash on the screen before it displays the stored file names. I'll fix that Friday morning, too.

[guardrex]

I converted the first example over something closer to your, @pranavkm, engineering repo example ...

https://github.com/dotnet/AspNetCore.Docs/blob/master/aspnetcore/blazor/file-uploads/samples/5.x/Components/FilePreview.razor

To shorten it generally, I removed the image bits. The images piece also had the base64 string piece that we don't want. I now remove the cross-link to the engineering repo to surface the FilePreview component.

btw - You have a StateHasChanged call in there just inside the foreach. I'm not sure why that's there, and I cut it. Let me know if that should go back into the example (and why ... I'll surface the reason to the reader).

Ignore the wrong label in the following image. I just fixed it ...

Upload up to @maxAllowedFiles files of up to @maxFileSize bytes each:

[guardrex]

To get rid of the pesky rendering problem, I took the ShouldRender() approach. It seems to work well enough, but let me know if that's a bad idea and how it should actually be addressed. The updates are now on the PR.

[guardrex]

@EdCharbeneau ... We're dropping the author bylines (dotnet/AspNetCore.Docs #21440) from all topics unless a non-FTE/non-vendor contributor wrote the topic or contributed heavily to it. Of course, you're welcome to make such contributions, and we'll provide a byline under those conditions.

[guardrex]

@pranavkm ... Latest code draft approach is on the PR.

• I pulled the base64 strings in favor of merely listing uploaded files.
• In the server example (2nd example), see if the ShouldRender approach makes sense to prevent an interim render during OnInputFileChange, which flashes the error markup on the screen for each file before files are finished uploading (i.e., FileUpload is false for each file during the initial render). The ShouldRender here resolves the problem ... is it sane? 😵

[guardrex]

@pranavkm @javiercn ...

• I reacted to your earlier feedback. I added text to explain a few aspects of the example. I don't think that this section requires a repeat of the earlier API descriptions. The security code is self-exclamatory, and there's a cross-link to our existing security guidance.
• I used ShouldRender to prevent an interim render during OnInputFileChange, without which the error markup flashes on the screen for each file before files are finished uploading. It seems to work well, but let me know if I went the wrong way there.

Anything else to consider on this one? If not, I'll give it a last look on Friday and get it published.