-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Fix IBrowserFile Name property to return FileName instead of form field name #63238
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
Conversation
Co-authored-by: javiercn <[email protected]>
…d tests Co-authored-by: javiercn <[email protected]>
You're absolutely right, I misunderstood the issue completely. I've now reverted my incorrect changes and applied the correct fix from PR #56833. The issue was that |
…me vs form field name Co-authored-by: javiercn <[email protected]>
I've added an E2E test |
...ents/test/E2ETest/ServerRenderingTests/FormHandlingTests/FormWithParentBindingContextTest.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: javiercn <[email protected]>
There was a problem hiding this comment.
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 fixes a bug in the BrowserFileFromFormFile
class where the Name
property was incorrectly returning the HTML form field name instead of the actual uploaded file name. The fix ensures that IBrowserFile.Name
returns the expected file name (e.g., "document.pdf") rather than the form input name (e.g., "fileInput").
Key changes:
- Changed the
Name
property implementation to useformFile.FileName
instead offormFile.Name
- Added comprehensive unit tests to verify the correct behavior for single files and file collections
- Updated existing tests that were expecting the incorrect behavior
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/Components/Endpoints/src/FormMapping/BrowserFileFromFormFile.cs |
Core fix: Changed Name property to return FileName instead of form field name |
src/Components/Endpoints/test/FormMapping/BrowserFileFromFormFileTests.cs |
New test file with comprehensive coverage for the Name property behavior |
src/Components/Endpoints/test/Binding/FormDataMapperTests.cs |
Updated existing tests to expect actual file names and fixed test logic bugs |
src/Components/test/E2ETest/ServerRenderingTests/FormHandlingTests/FormWithParentBindingContextTest.cs |
Updated E2E test assertion to use actual file name |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
var browserFile2 = browserFiles[0]; | ||
Assert.Equal("file", browserFile2.Name); | ||
Assert.Equal(expectedString1.Length, browserFile2.Size); | ||
var browserFile2 = browserFiles[1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line correctly fixes the bug where browserFile2 was incorrectly assigned to browserFiles[0] instead of browserFiles[1], but the change appears to be fixing a pre-existing test bug rather than being directly related to the Name property fix.
Copilot uses AI. Check for mistakes.
var buffer2 = new byte[browserFile2.Size]; | ||
browserFile1.OpenReadStream().Read(buffer2); | ||
Assert.Equal(expectedString1, Encoding.UTF8.GetString(buffer2, 0, buffer2.Length)); | ||
browserFile2.OpenReadStream().Read(buffer2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line correctly fixes the bug where the test was incorrectly reading from browserFile1's stream instead of browserFile2's stream when testing the second file.
Copilot uses AI. Check for mistakes.
When binding form fields with
Input Type=File
toIBrowserFile
, theBrowserFileFromFormFile
class was incorrectly returning the form field name instead of the actual uploaded file name.The issue was in the
Name
property implementation:This would return the HTML form field name instead of the actual uploaded file name, causing a mismatch between the expected file name and what was returned.
The fix changes the mapping to use the correct property:
This change:
IBrowserFile.Name
Added comprehensive tests covering the
Name
property mapping for both single files and file collections, updated existing tests that were expecting the incorrect behavior, and added an E2E test that validates the fix in server-rendered forms.Fixes #56832.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.