[Windows]Fix NullReferenceException in OpenReadAsync for FileResult created with full path#28238
Conversation
| _ = await stream.ReadAsync(bytes, 0, (int)stream.Length); | ||
|
|
||
| Assert.True(bytes.Length > 0); |
There was a problem hiding this comment.
This caught my attention. Should this be:
| _ = await stream.ReadAsync(bytes, 0, (int)stream.Length); | |
| Assert.True(bytes.Length > 0); | |
| var count = await stream.ReadAsync(bytes, 0, (int)stream.Length); | |
| Assert.True(count > 0); |
?
bytes.Lenght is always >0, or not?
(I did not open the PR in my IDE. Sorry, if I'm confusing things.)
There was a problem hiding this comment.
@MartyIX, I’ve removed the assert that check bytes.Length and completed the test using Assert.NotNull(stream)
|
|
||
| Assert.NotNull(stream); | ||
|
|
||
| File.Delete(filePath); |
There was a problem hiding this comment.
L55 (using var stream = await fileResult.OpenReadAsync();) is still using the file..?
There was a problem hiding this comment.
@MartyIX,
It is using FileResult, not the file directly. The OpenReadAsync() method returns a stream from the FileResult
There was a problem hiding this comment.
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 stream is disposed.
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.
Hello,
does now the FileResult is able to load the file correctly ? i'm in maui net 8
Thank's
There was a problem hiding this comment.
@robitsrl , Yes, in .NET 8, FileResult is able to load the file correctly when creating a new FileResult using an existing FileResult.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
|
@devanathan-vaithiyanathan Could you rebase and fix the conflict? |
964bcdc to
07c0503
Compare
@jsuarezruiz , I’ve rebased the branch and resolved the conflicts. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| File.Delete(filePath); | ||
| } | ||
| #endif | ||
| [Fact] |
There was a problem hiding this comment.
Could add a test that calls OpenReadAsync twice 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.
@jsuarezruiz , Based on suggestion, I've modified the test
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where FileResult.OpenReadAsync() fails on UWP when a FileResult is created with only a file path (without a StorageFile object). The fix enables the UWP implementation to lazily load the StorageFile from the path when needed.
- Added lazy
StorageFileinitialization in UWP'sPlatformOpenReadAsyncmethod - Added tests to verify
FileResultworks with file paths and supports multiple stream openings
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Essentials/src/FileSystem/FileSystem.uwp.cs | Updated PlatformOpenReadAsync to lazily initialize StorageFile from FullPath when File is null |
| src/Essentials/test/DeviceTests/Tests/FileSystem_Tests.cs | Added two new tests to verify FileResult works with file paths and supports multiple OpenReadAsync calls |
| var fileResult = new FileResult(filePath); | ||
|
|
||
| using var stream = await fileResult.OpenReadAsync(); | ||
|
|
||
| Assert.NotNull(stream); | ||
|
|
||
| File.Delete(filePath); |
There was a problem hiding this comment.
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.
| 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); | |
| } |
| string filePath = Path.Combine(FileSystem.CacheDirectory, "sample_multiple.txt"); | ||
| string expectedContent = "Sample content for multiple stream testing"; | ||
| await File.WriteAllTextAsync(filePath, expectedContent); | ||
|
|
||
| var fileResult = new FileResult(filePath); | ||
|
|
||
| // First call to OpenReadAsync | ||
| using (var firstStream = await fileResult.OpenReadAsync()) | ||
| { | ||
| Assert.NotNull(firstStream); | ||
| using var firstReader = new StreamReader(firstStream); | ||
| var firstContent = await firstReader.ReadToEndAsync(); | ||
| Assert.Equal(expectedContent, firstContent); | ||
| } | ||
|
|
||
| // Second call to OpenReadAsync - should still work | ||
| using (var secondStream = await fileResult.OpenReadAsync()) | ||
| { | ||
| Assert.NotNull(secondStream); | ||
| using var secondReader = new StreamReader(secondStream); | ||
| var secondContent = await secondReader.ReadToEndAsync(); | ||
| Assert.Equal(expectedContent, secondContent); | ||
| } | ||
|
|
||
| File.Delete(filePath); |
There was a problem hiding this comment.
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.
5800c87 to
43ea2ef
Compare
@PureWeen , I've resolved the merge conflicts |
…sLayout is set for Tablet but not on mobile devices (dotnet#26152)" This reverts commit 0ddc794.
…msLayout is set for Tablet but not on mobile devices (dotnet#26152)" This reverts commit 4b9074c.
This reverts commit c68ce9e.
22c1b4b to
2f6da15
Compare
Merge conflicts have been resolved by the author
StephaneDelcroix
left a comment
There was a problem hiding this comment.
LGTM! The fix correctly handles the case where a FileResult is created with just a path by lazily loading the StorageFile when needed. The tests cover both single and multiple OpenReadAsync calls. The CI failures appear to be unrelated flaky tests (Layout, WebView).
The base branch was changed.
…reated with full path (#28238) ### Issue details Calling OpenReadAsync() on a FileResult constructed using the full path constructor throws a NullReferenceException with the message: "Value cannot be null. (Parameter 'windowsRuntimeFile')". This occurs because the internal File property is null when a full path is provided. ### Description of Change The changes fix the Exception in OpenReadAsync() by properly initializing the File property when a FileResult is created with a full path. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #26858 **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Windows**<br> <img src="https://github.com/user-attachments/assets/d0fd9a0f-7b7a-4c15-a15f-69f978060067" > |**Windows**<br> <img src="https://github.com/user-attachments/assets/0059d3d6-6350-487a-a8be-618cc6221b1e" > | --------- Co-authored-by: Shalini-Ashokan <102292178+Shalini-Ashokan@users.noreply.github.com>
…reated with full path (#28238) ### Issue details Calling OpenReadAsync() on a FileResult constructed using the full path constructor throws a NullReferenceException with the message: "Value cannot be null. (Parameter 'windowsRuntimeFile')". This occurs because the internal File property is null when a full path is provided. ### Description of Change The changes fix the Exception in OpenReadAsync() by properly initializing the File property when a FileResult is created with a full path. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #26858 **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Windows**<br> <img src="https://github.com/user-attachments/assets/d0fd9a0f-7b7a-4c15-a15f-69f978060067" > |**Windows**<br> <img src="https://github.com/user-attachments/assets/0059d3d6-6350-487a-a8be-618cc6221b1e" > | --------- Co-authored-by: Shalini-Ashokan <102292178+Shalini-Ashokan@users.noreply.github.com>
…reated with full path (#28238) ### Issue details Calling OpenReadAsync() on a FileResult constructed using the full path constructor throws a NullReferenceException with the message: "Value cannot be null. (Parameter 'windowsRuntimeFile')". This occurs because the internal File property is null when a full path is provided. ### Description of Change The changes fix the Exception in OpenReadAsync() by properly initializing the File property when a FileResult is created with a full path. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #26858 **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Windows**<br> <img src="https://github.com/user-attachments/assets/d0fd9a0f-7b7a-4c15-a15f-69f978060067" > |**Windows**<br> <img src="https://github.com/user-attachments/assets/0059d3d6-6350-487a-a8be-618cc6221b1e" > | --------- Co-authored-by: Shalini-Ashokan <102292178+Shalini-Ashokan@users.noreply.github.com>
…reated with full path (#28238) ### Issue details Calling OpenReadAsync() on a FileResult constructed using the full path constructor throws a NullReferenceException with the message: "Value cannot be null. (Parameter 'windowsRuntimeFile')". This occurs because the internal File property is null when a full path is provided. ### Description of Change The changes fix the Exception in OpenReadAsync() by properly initializing the File property when a FileResult is created with a full path. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #26858 **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Windows**<br> <img src="https://github.com/user-attachments/assets/d0fd9a0f-7b7a-4c15-a15f-69f978060067" > |**Windows**<br> <img src="https://github.com/user-attachments/assets/0059d3d6-6350-487a-a8be-618cc6221b1e" > | --------- Co-authored-by: Shalini-Ashokan <102292178+Shalini-Ashokan@users.noreply.github.com>
…reated with full path (#28238) ### Issue details Calling OpenReadAsync() on a FileResult constructed using the full path constructor throws a NullReferenceException with the message: "Value cannot be null. (Parameter 'windowsRuntimeFile')". This occurs because the internal File property is null when a full path is provided. ### Description of Change The changes fix the Exception in OpenReadAsync() by properly initializing the File property when a FileResult is created with a full path. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #26858 **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Windows**<br> <img src="https://github.com/user-attachments/assets/d0fd9a0f-7b7a-4c15-a15f-69f978060067" > |**Windows**<br> <img src="https://github.com/user-attachments/assets/0059d3d6-6350-487a-a8be-618cc6221b1e" > | --------- Co-authored-by: Shalini-Ashokan <102292178+Shalini-Ashokan@users.noreply.github.com>
…reated with full path (#28238) ### Issue details Calling OpenReadAsync() on a FileResult constructed using the full path constructor throws a NullReferenceException with the message: "Value cannot be null. (Parameter 'windowsRuntimeFile')". This occurs because the internal File property is null when a full path is provided. ### Description of Change The changes fix the Exception in OpenReadAsync() by properly initializing the File property when a FileResult is created with a full path. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #26858 **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Windows**<br> <img src="https://github.com/user-attachments/assets/d0fd9a0f-7b7a-4c15-a15f-69f978060067" > |**Windows**<br> <img src="https://github.com/user-attachments/assets/0059d3d6-6350-487a-a8be-618cc6221b1e" > | --------- Co-authored-by: Shalini-Ashokan <102292178+Shalini-Ashokan@users.noreply.github.com>
…reated with full path (#28238) ### Issue details Calling OpenReadAsync() on a FileResult constructed using the full path constructor throws a NullReferenceException with the message: "Value cannot be null. (Parameter 'windowsRuntimeFile')". This occurs because the internal File property is null when a full path is provided. ### Description of Change The changes fix the Exception in OpenReadAsync() by properly initializing the File property when a FileResult is created with a full path. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #26858 **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Windows**<br> <img src="https://github.com/user-attachments/assets/d0fd9a0f-7b7a-4c15-a15f-69f978060067" > |**Windows**<br> <img src="https://github.com/user-attachments/assets/0059d3d6-6350-487a-a8be-618cc6221b1e" > | --------- Co-authored-by: Shalini-Ashokan <102292178+Shalini-Ashokan@users.noreply.github.com>
## What's Coming .NET MAUI inflight/candidate introduces significant improvements across all platforms with focus on quality, performance, and developer experience. This release includes 16 commits with various improvements, bug fixes, and enhancements. ## Checkbox - [Android] Implement material3 support for CheckBox by @HarishwaranVijayakumar in #33339 <details> <summary>🔧 Fixes</summary> - [Implement Material3 Support for CheckBox](#33338) </details> ## CollectionView - [Android] Fixed EmptyView doesn’t display when CollectionView is placed inside a VerticalStackLayout by @NanthiniMahalingam in #33134 <details> <summary>🔧 Fixes</summary> - [CollectionView does not show an EmptyView template with an empty collection](#32932) </details> ## Essentials - [Windows]Fix NullReferenceException in OpenReadAsync for FileResult created with full path by @devanathan-vaithiyanathan in #28238 <details> <summary>🔧 Fixes</summary> - [[Windows] FileResult(string fullPath) not initialized properly](#26858) </details> ## Image - Fix Glide IllegalArgumentException in MauiCustomTarget.clear() for destroyed activities by @jfversluis via @Copilot in #29780 <details> <summary>🔧 Fixes</summary> - [java.lang.IllegalArgumentException: You cannot start a load for a destroyed activity - glide](#29699) </details> ## Label - [Android] Fix for Label WordWrap width issue causing HorizontalOptions misalignment by @praveenkumarkarunanithi in #33281 <details> <summary>🔧 Fixes</summary> - [[Android] Unexpected Line Breaks in Android, Label with WordWrap Mode Due to Trailing Space.](#31782) - [Label not sized correctly on Android](#27614) </details> - Fix to Improve Flyout Accessibility by Adjusting UITableViewController Labels by @SuthiYuvaraj in #31619 <details> <summary>🔧 Fixes</summary> - [Navigation section present under hamburger are programmatically define as table :A11y_.NET maui_User can get all the insights of Dashboard_Devtools](#30894) </details> ## Mediapicker - [Regression][iOS] Fix MediaPicker PickPhotosAsync getting file name in contentType property by @devanathan-vaithiyanathan in #33390 <details> <summary>🔧 Fixes</summary> - [[iOS] MediaPicker PickPhotosAsync getting file name in contentType property](#33348) </details> ## Navigation - Fix handler not disconnected when removing non visible pages using RemovePage() by @Vignesh-SF3580 in #32289 <details> <summary>🔧 Fixes</summary> - [NavigationPage.Navigation.RemovePage() fails to disconnect handlers when removing pages, unlike ContentPage.Navigation.RemovePage()](#32239) </details> ## Picker - [Android] Fix Picker IsOpen not reset when picker is dismissed by @devanathan-vaithiyanathan in #33332 <details> <summary>🔧 Fixes</summary> - [[Android] Picker IsOpen not reset when picker is dismissed](#33331) </details> ## Shell - [iOS & Catalyst ] Fixed IsEnabled property should work on Tabs by @SubhikshaSf4851 in #33369 <details> <summary>🔧 Fixes</summary> - [[Catalyst] TabBarBackgroundColor, TabBarUnselectedColor, and IsEnabled Not Working as Expected in Shell](#33158) </details> - [iOS,Windows] Fix navigation bar colors not resetting when switching ShellContent by @Vignesh-SF3580 in #33228 <details> <summary>🔧 Fixes</summary> - [[iOS, Windows] Shell Navigation bar colors are not updated correctly when switching ShellContent](#33227) </details> - [iOS] Fixed Shell navigation on search handler suggestion selection by @SubhikshaSf4851 in #33406 <details> <summary>🔧 Fixes</summary> - [[iOS] Clicking on search suggestions fails to navigate to detail page correctly](#33356) </details> ## Templates - Fix VoiceOver doesnot announces the State of the ComboBox by @SuthiYuvaraj in #32286 ## Xaml - [XSG][BindingSourceGen] Add support for CommunityToolkit.Mvvm ObservablePropertyAttribute by @simonrozsival via @Copilot in #33028 <details> <summary>🔧 Fixes</summary> - [[XSG] Add heuristic to support bindable properties generated by other source generators](#32597) </details> <details> <summary>📦 Other (2)</summary> - [XSG] Improve diagnostic reporting during binding compilation by @simonrozsival via @Copilot in #32905 - [Testing] Fixed Test case failure in PR 33574 - [01/19/2026] Candidate - 1 by @TamilarasanSF4853 in #33602 </details> **Full Changelog**: main...inflight/candidate
Issue details
Calling OpenReadAsync() on a FileResult constructed using the full path constructor throws a NullReferenceException with the message: "Value cannot be null. (Parameter 'windowsRuntimeFile')". This occurs because the internal File property is null when a full path is provided.
Description of Change
The changes fix the Exception in OpenReadAsync() by properly initializing the File property when a FileResult is created with a full path.
Issues Fixed
Fixes #26858
Tested the behavior in the following platforms.