fix(v3/macOS): prevent OpenFile dialog crash when UTType cannot resolve filter extensions#5678
fix(v3/macOS): prevent OpenFile dialog crash when UTType cannot resolve filter extensions#5678phergul wants to merge 4 commits into
Conversation
for compound extensions like "tar.gz", pathExtension only returns the
last segment ("gz")
WalkthroughmacOS open-file dialog filtering now splits recognized and unrecognized filename extensions, and the delegate now validates allowed files by matching the lowercase filename suffix instead of relying on the URL path extension. ChangesmacOS open-file dialog filtering
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Amazing! Thanks 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@v3/pkg/application/dialogs_darwin_delegate.m`:
- Around line 29-34: Normalize allowed extension values in the Darwin dialog
filtering path by stripping any leading dots before they reach the matching
logic, since allowedExtensions can still contain entries like .png that will
never match the hasSuffix check or resolve correctly through UTType
typeWithFilenameExtension. Update the normalization in the dialog setup and keep
the checks in dialogs_darwin_delegate.m consistent with the cleaned extension
values so both suffix matching and type resolution use the same canonical form.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b5385b82-9234-41f3-b0c5-91d6c4ec49c8
📒 Files selected for processing (2)
v3/pkg/application/dialogs_darwin.gov3/pkg/application/dialogs_darwin_delegate.m
| for (NSString *allowedExt in self.allowedExtensions) { | ||
| if ([[allowedExt lowercaseString] isEqualToString:extension]) { | ||
| NSString *allowed = [allowedExt lowercaseString]; | ||
| if ([allowed length] == 0) { | ||
| continue; | ||
| } | ||
| if ([filename hasSuffix:[@"." stringByAppendingString:allowed]]) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -a 'dialogs*' v3/pkg/application
rg -n -C4 '\ballowedExtensions\b' v3/pkg/application
rg -n -C4 'typeWithFilenameExtension|setAllowedFileTypes|setAllowedContentTypes|hasSuffix' v3/pkg/applicationRepository: wailsapp/wails
Length of output: 8077
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Darwin dialog path and the upstream filter normalization.
sed -n '1,260p' v3/pkg/application/dialogs_darwin.go
printf '\n---\n'
sed -n '1,260p' v3/pkg/application/dialogs.go
printf '\n---\n'
sed -n '1,260p' v3/pkg/application/dialogs_test.go
printf '\n---\n'
rg -n -C3 'filterPatterns|allowedExtensions|AllowedFileTypes|AllowedContentTypes|Filter' v3/pkg/applicationRepository: wailsapp/wails
Length of output: 38141
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '486,508p' v3/pkg/application/dialogs_darwin.go
printf '\n---\n'
rg -n -C4 'TrimPrefix|TrimSpace|strings\.Join|patternComponents|filterPatterns' v3/pkg/application/dialogs_darwin.goRepository: wailsapp/wails
Length of output: 3562
Strip leading dots during Darwin filter normalization.
dialogs_darwin.go trims *. but leaves values like .png unchanged, so allowedExtensions can still contain entries that never match the ." + allowed check or resolve cleanly through UTType typeWithFilenameExtension:.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@v3/pkg/application/dialogs_darwin_delegate.m` around lines 29 - 34, Normalize
allowed extension values in the Darwin dialog filtering path by stripping any
leading dots before they reach the matching logic, since allowedExtensions can
still contain entries like .png that will never match the hasSuffix check or
resolve correctly through UTType typeWithFilenameExtension. Update the
normalization in the dialog setup and keep the checks in
dialogs_darwin_delegate.m consistent with the cleaned extension values so both
suffix matching and type resolution use the same canonical form.
2 participants
Description
Fixes a crash on macOS when a file dialog filter contains extensions that
UTTypecannot resolveshowOpenFileDialogbuildsallowedContentTypesby callingUTType typeWithFilenameExtensionfor each filter extension. That API returns nil when it is not a 'recognised' UTType. Inserting nil into theNSMutableArraythrowsNSInvalidArgumentExceptionand crashes the app.dialogs_darwin.go: Skip nil UTTypes and add unrecognized extensions to the legacysetAllowedFileTypesAPI (same path that is used for macOS < 11). Recognized extensions continue to usesetAllowedContentTypes.dialogs_darwin_delegate.m: Match files by filename suffix instead of pathExtension, so compound extensions like tar.gz are selectable (pathExtensiononly returns gz for example.tar.gz).I checked how Electron handles this, and they simply take only the last extension from the users input (e.g. .tar.gz -> .gz) and filter on that. Comparing using the filename suffix lets matching be more accurate.
Fixes #5675
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Windows
macOS
Linux
go test ./...
go mod replace with branch, tested the unrecognised extensions (tar.gz, busted.exe). Also tested with full list of both recognised and unrecognised ones.
Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PR (v3 changelog entries are added automatically)Summary by CodeRabbit