-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Windows]Fix NullReferenceException in OpenReadAsync for FileResult created with full path #28238
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
Changes from all commits
d536482
dd1c4ad
0379f09
dceea4b
0fe4caf
63265d1
c889941
feba053
b84df2f
0d3cbb5
a571857
eb0e699
2f6da15
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -57,5 +57,49 @@ public async Task ValidateMIMEFormat() | |||||||||||||||||||||||||||||||||||||||
| File.Delete(filePath); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| #endif | ||||||||||||||||||||||||||||||||||||||||
| [Fact] | ||||||||||||||||||||||||||||||||||||||||
| public async Task CheckFileResultWithFilePath() | ||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||
| string filePath = Path.Combine(FileSystem.CacheDirectory, "sample.txt"); | ||||||||||||||||||||||||||||||||||||||||
| await File.WriteAllTextAsync(filePath, "Sample content for testing"); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| var fileResult = new FileResult(filePath); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| using var stream = await fileResult.OpenReadAsync(); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| Assert.NotNull(stream); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| File.Delete(filePath); | ||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. L55 (
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MartyIX,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know this API in depth. It's just I find it weird that you seem to operate on a file and delete it before the I would expect: using (var stream = await fileResult.OpenReadAsync())
{
Assert.NotNull(stream);
}
File.Delete(filePath);I find it safer. But I might be very well wrong, so feel free to ignore this suggestion. edit: Ah, I see what you are saying. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @robitsrl , Yes, in .NET 8, FileResult is able to load the file correctly when creating a new FileResult using an existing FileResult.
Comment on lines
+66
to
+72
|
||||||||||||||||||||||||||||||||||||||||
| var fileResult = new FileResult(filePath); | |
| using var stream = await fileResult.OpenReadAsync(); | |
| Assert.NotNull(stream); | |
| File.Delete(filePath); | |
| try | |
| { | |
| var fileResult = new FileResult(filePath); | |
| using var stream = await fileResult.OpenReadAsync(); | |
| Assert.NotNull(stream); | |
| } | |
| finally | |
| { | |
| File.Delete(filePath); | |
| } |
Copilot
AI
Nov 5, 2025
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.
The file cleanup could fail if an exception occurs during the test. Consider wrapping the test logic in a try-finally block or moving the cleanup to a finally block to ensure the test file is deleted even if assertions fail.
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.
Could add a test that calls
OpenReadAsynctwice to validate it? Ensures the second call does not regress and that the StorageFile remains usable across multiple streams.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.
@jsuarezruiz , Based on suggestion, I've modified the test