Maui
Maui copied to clipboard
Fix IOS crash on invalid or missing file extension on Resource Media Source
- Bug fix
Description of Change
The primary change made to the code involves enhancing the validation of the path
variable. Previously, the code checked if the path
was neither null nor consisted solely of whitespace. The update extends this validation by also verifying that the path
includes an extension. This adjustment is crucial for ensuring that operations such as retrieving the directory name, the filename without its extension, and the file extension itself are only executed on valid file paths. This modification aims to mitigate potential errors or unexpected outcomes when the code encounters paths that do not correspond to actual files, which would characteristically include extensions.
Summary of Changes:
- Enhanced validation of the
path
variable to include a check for the presence of a file extension, in addition to the existing checks for null or whitespace content. This ensures that operations reliant on the path being a file (with an extension) are only performed on suitable paths, thereby improving the robustness of the code against invalid path inputs.
Reference to Code Changes:
- The condition for checking the
path
variable has been updated to include a check for a file extension.
Linked Issues
- Fixes #1856
PR Checklist
- [x] Has a linked Issue, and the Issue has been
approved
(bug) orChampioned
(feature/proposal) - [ ] Has tests (if omitted, state reason in description)
- [ ] Has samples (if omitted, state reason in description)
- [x] Rebased on top of
main
at time of PR - [x] Changes adhere to coding standard
- [ ] Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls
Additional information
Fixes issue with invalid or missing file extension for video source that caused BSOD on IOS when trying to set source without a file extension.
How do the other platforms behave? Is the result of this PR that the setting of the resource silently fails? If so is that the desired behavior? If we are receiving an invalid source I think an appropriate exception or trace log would be a good option
How do the other platforms behave? Is the result of this PR that the setting of the resource silently fails? If so is that the desired behavior? If we are receiving an invalid source I think an appropriate exception or trace log would be a good option
This fix only applies to IOS and should have zero impact on any other platform. The code is only in the macios branch and should not be included in any other platform. As to how other platform behave I have not tested. I'm sure someone can file a bug report if there is an issue. With this PR I am focused on the issue that was reported and anything else is outside of scope.
Edit: I added a log message on error about invalid file paths if it fails.
This fix only applies to IOS and should have zero impact on any other platform. The code is only in the macios branch and should not be included in any other platform. As to how other platform behave I have not tested. I'm sure someone can file a bug report if there is an issue. With this PR I am focused on the issue that was reported and anything else is outside of scope.
Edit: I added a log message on error about invalid file paths if it fails.
Thanks, my question on how the other platforms behave was not to suggest we fix those also but to make sure that we are being consistent across all platforms. If Android throws an exception then I don't think iOS should silently fail.
I haven't been able to test on Android as my emulators/SDK is messed up so I might be sending us down the wrong path. I just want to make sure that we are considering the implication of the change.
I also don't see an issue with throwing an exception. Now it should be a specific exception providing a suitable message to help developers.
This fix only applies to IOS and should have zero impact on any other platform. The code is only in the macios branch and should not be included in any other platform. As to how other platform behave I have not tested. I'm sure someone can file a bug report if there is an issue. With this PR I am focused on the issue that was reported and anything else is outside of scope. Edit: I added a log message on error about invalid file paths if it fails.
Thanks, my question on how the other platforms behave was not to suggest we fix those also but to make sure that we are being consistent across all platforms. If Android throws an exception then I don't think iOS should silently fail.
I haven't been able to test on Android as my emulators/SDK is messed up so I might be sending us down the wrong path. I just want to make sure that we are considering the implication of the change.
I also don't see an issue with throwing an exception. Now it should be a specific exception providing a suitable message to help developers.
On both windows and android the OS itself outputs an error message saying something about an invalid file. It does nothing else. So with the changes to IOS it will be the same behavior across all platforms. Except tizen which I don't have the ability to test.
Ok great then ignore my previous comments. Thanks for the effort in checking that