-
Notifications
You must be signed in to change notification settings - Fork 92
Fix win 11 zip files erroring when installing into mod manager #1246
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
📝 WalkthroughWalkthroughChanged ZIP extraction logic in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
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 |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
OpenKh.Tools.ModsManager/Services/ModsService.cs (1)
245-245: Update this filter to match the new logic at line 176.Line 245 still uses the old
ExternalAttributes-based directory filtering that was replaced at line 176. If Windows 11 ZIP files have incorrectExternalAttributes(which motivated the fix at line 176), this section will also fail or produce incorrect metadata.For consistency and correctness, update this filter to match line 176's approach:
🔧 Proposed fix to align filtering logic
-foreach (var entry in zipFile.Entries.Where(x => (x.ExternalAttributes & 0x10) != 0x10)) +foreach (var entry in zipFile.Entries.Where(x => !string.IsNullOrEmpty(x.Name)))Or use the more robust version if you adopt the suggested improvement from line 176:
-foreach (var entry in zipFile.Entries.Where(x => (x.ExternalAttributes & 0x10) != 0x10)) +foreach (var entry in zipFile.Entries.Where(x => !string.IsNullOrEmpty(x.Name) && !x.FullName.EndsWith("/") && !x.FullName.EndsWith("\\")))
🧹 Nitpick comments (1)
OpenKh.Tools.ModsManager/Services/ModsService.cs (1)
176-176: Verify that Name-based filtering reliably excludes all directory entries.The change from
ExternalAttributes-based filtering to checking!string.IsNullOrEmpty(x.Name)may resolve issues with Windows 11 ZIP files, but could be fragile across different ZIP implementations. Directory entries typically have an emptyNameproperty, but this isn't guaranteed by the ZIP specification.Consider a more robust check that also verifies
FullNamedoesn't end with a path separator:foreach (var entry in zipFile.Entries.Where(x => !string.IsNullOrEmpty(x.Name) && !x.FullName.EndsWith("/") && !x.FullName.EndsWith("\\")))This would prevent attempting to create a file (line 199) for any directory entries that slip through with a non-empty
Name.Please verify this fix with actual Windows 11 ZIP files to ensure:
- Directory entries are correctly excluded
- All expected file entries are extracted
- No
File.Createerrors occur on directory paths
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
OpenKh.Tools.ModsManager/Services/ModsService.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (csharp)
- GitHub Check: build
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.