fix #1514 Замена dotnetzip на другой форк#1558
Conversation
ProDotNetZip и его родитель DotNetZip.Semverd имеют коммит с исправлением: haf/DotNetZip.Semverd@18486ad
WalkthroughThe changes update ZIP file handling by replacing the DotNetZip library with ProDotNetZip, introducing a new thread-safe mechanism for setting ZIP encoding, and improving encoding detection logic. Test files are adjusted for better diagnostics and to account for known library issues. Minor project and attribute configuration updates are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant ZipReader
participant DotNetZipEncoding
participant ZipFile
ZipReader->>DotNetZipEncoding: SetDefault(Encoding.GetEncoding(866))
DotNetZipEncoding->>ZipFile: Set DefaultEncoding (thread-safe, once)
ZipReader->>ZipFile: Open ZIP file with default encoding
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/OneScript.StandardLibrary/Zip/DotNetZipEncoding.cs (2)
20-23: Упростите конструктор.Инициализация
_encodingIsSet = falseизбыточна, так как это значение по умолчанию дляbool. Конструктор можно убрать.- static DotNetZipEncoding() - { - _encodingIsSet = false; - }
45-60: Рассмотрите удаление неиспользуемого кода.Метод
SetDefaultEncodingViaReflectionопределен но не используется. Если он предназначен как резервный вариант для других версий библиотеки, добавьте комментарий с объяснением. Иначе рассмотрите его удаление.src/OneScript.StandardLibrary/Zip/ZipWriter.cs (1)
55-55: Отличная интеграция с новым механизмом управления кодировками.Использование
DotNetZipEncoding.SetDefault()вместо прямого присваивания обеспечивает потокобезопасность и централизованное управление кодировкой.Рассмотрите улучшение комментария:
- DotNetZipEncoding.SetDefault(Encoding.GetEncoding(866)); // fuck non-russian encodings on non-ascii files + DotNetZipEncoding.SetDefault(Encoding.GetEncoding(866)); // default encoding for non-ASCII file names
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/zip/fixtures/testUTF8.zipis excluded by!**/*.zip
📒 Files selected for processing (8)
.gitattributes(1 hunks)src/OneScript.StandardLibrary/OneScript.StandardLibrary.csproj(1 hunks)src/OneScript.StandardLibrary/Text/TextEncodingEnum.cs(2 hunks)src/OneScript.StandardLibrary/Zip/DotNetZipEncoding.cs(1 hunks)src/OneScript.StandardLibrary/Zip/ZipReader.cs(2 hunks)src/OneScript.StandardLibrary/Zip/ZipWriter.cs(1 hunks)tests/native-api.os(1 hunks)tests/zip/test-zip.os(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: EvilBeaver
PR: EvilBeaver/OneScript#0
File: :0-0
Timestamp: 2024-08-05T18:27:08.563Z
Learning: For each new pull request in the EvilBeaver/OneScript repository, include a poem in Russian.
Learnt from: EvilBeaver
PR: EvilBeaver/OneScript#1557
File: src/VSCode.DebugAdapter/Transport/RpcOperationException.cs:11-11
Timestamp: 2025-07-18T12:25:24.078Z
Learning: EvilBeaver prefers to preserve original namespaces for types involved in binary serialization or cross-version compatibility, even if the type is not directly serialized, to maintain overall system compatibility.
Learnt from: EvilBeaver
PR: EvilBeaver/OneScript#1557
File: src/VSCode.DebugAdapter/Transport/DebugClientFactory.cs:60-0
Timestamp: 2025-07-16T12:44:12.167Z
Learning: EvilBeaver prefers fail-fast error handling in debugging contexts - when stream operations fail in debug client factories, exceptions should be thrown to terminate the process rather than handled gracefully with fallback mechanisms.
Learnt from: EvilBeaver
PR: EvilBeaver/OneScript#1555
File: Build.csproj:35-38
Timestamp: 2025-07-10T11:08:13.756Z
Learning: EvilBeaver prefers to see documentation links when technical claims are made about MSBuild syntax and property functions.
📚 Learning: в репозитории evilbeaver/onescript всегда нужно говорить по-русски....
Learnt from: nixel2007
PR: EvilBeaver/OneScript#0
File: :0-0
Timestamp: 2024-08-28T16:51:21.322Z
Learning: В репозитории EvilBeaver/OneScript всегда нужно говорить по-русски.
Applied to files:
tests/zip/test-zip.os
📚 Learning: в коде кэширования скриптов onescript файлы кэша и метаданных создаются в той же директории, что и и...
Learnt from: EvilBeaver
PR: EvilBeaver/OneScript#1555
File: src/ScriptEngine.HostedScript/LibraryCache/FileSystemScriptCache.cs:158-166
Timestamp: 2025-07-10T11:06:46.818Z
Learning: В коде кэширования скриптов OneScript файлы кэша и метаданных создаются в той же директории, что и исходный файл, поэтому если исходный файл существует, то директория гарантированно существует и дополнительные проверки не нужны.
Applied to files:
tests/zip/test-zip.os
🔇 Additional comments (13)
tests/native-api.os (1)
53-53: Отличное улучшение диагностики!Добавление полного пути к сообщению об ошибке значительно упростит отладку при отсутствии прокси-библиотеки NativeApi.
src/OneScript.StandardLibrary/Zip/DotNetZipEncoding.cs (1)
25-38: LGTM! Отличная реализация double-checked locking.Правильное использование паттерна double-checked locking с volatile полем обеспечивает потокобезопасность и производительность.
.gitattributes (1)
4-4: No large .zip files detected; disabling LFS is safe
- Only one .zip present:
tests/zip/fixtures/testUTF8.zip(~196 B)
Disabling Git LFS for.zipfiles won’t meaningfully bloat the repository.src/OneScript.StandardLibrary/OneScript.StandardLibrary.csproj (1)
28-28: Подтвердите совместимость API ProDotNetZipЗаменили DotNetZip на ProDotNetZip (v1.20.0). Код ссылается на Ionic.Zip и ZipFile.* в следующих файлах:
- src/OneScript.StandardLibrary/NativeApi/NativeApiPackage.cs
- src/OneScript.StandardLibrary/Zip/ZipFileEntriesCollection.cs
- src/OneScript.StandardLibrary/Zip/ZipFileEntryContext.cs
- src/OneScript.StandardLibrary/Zip/DotNetZipEncoding.cs
- src/OneScript.StandardLibrary/Zip/ZipReader.cs
- src/OneScript.StandardLibrary/Zip/ZipWriter.cs
Убедитесь, что ProDotNetZip сохраняет прежний API (DefaultEncoding, Read/Write, AlternateEncoding и отражение поля _defaultEncoding), и прогоните интеграционные тесты. Если никаких несовместимостей не обнаружится, можно смело мёржить изменения.
src/OneScript.StandardLibrary/Zip/ZipReader.cs (2)
55-55: LGTM! Improved encoding management.The change from directly setting
ZipFile.DefaultEncodingto usingDotNetZipEncoding.SetDefault()centralizes encoding configuration and likely provides thread-safe initialization. This aligns well with the library replacement objective.
107-112: LGTM! Added necessary null validation.The null check for the
entryparameter prevents runtime exceptions and provides a more descriptive error message. The TODO comment indicates awareness of broader marshaling improvements needed.tests/zip/test-zip.os (4)
34-36: Временное отключение теста оправдано.Отключение теста Unicode имен файлов с ссылкой на конкретную проблему (#870) - правильное решение до исправления библиотеки.
108-109: Исправлена ошибка в утверждениях теста.Правильно исправлены имена файлов с "zip.os" на "test-zip.os" для соответствия фактическому имени тестового файла.
360-360: Исправлен путь к тестовому файлу.Изменение пути с "testlib" на "../testlib" обеспечивает правильное расположение русского файла для тестирования.
710-727: Реализация Unicode теста готова к использованию.Тест правильно извлекает файлы из UTF-8 архива с корректной обработкой исключений. Готов к активации после исправления библиотеки.
src/OneScript.StandardLibrary/Text/TextEncodingEnum.cs (3)
58-58: Улучшена надежность определения кодировки OEM.Добавление проверки
encoding.CodePage == 866обеспечивает корректное распознавание кодировки даже при различных экземплярах Encoding.
61-61: Улучшена надежность определения кодировки ANSI.Добавление проверки
encoding.CodePage == 1251повышает надежность распознавания кодировки Windows-1251.
76-76: Улучшена диагностика ошибок.Передача объекта
encodingв исключение обеспечивает лучший контекст для отладки неизвестных кодировок.
1 participant
Заменена реализация DotNetZip на ту в которой уязвимость отсутствует.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores