feat: BSL Language Server downloader#72
Conversation
Портирован из vsc-language-1c-bsl: скачивание ассета
bsl-language-server_{win,mac,nix}.zip последнего релиза или pre-release,
кэш по версии с SERVER-INFO, распаковка с сохранением unix-прав и
определение пути к исполняемому файлу сервера. Токен GitHub опционален.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VEHLaBE3PCoXkWraXA32Zp
…logs - Move BslLanguageServerDownloader/BslLanguageServerReleaseChannel to com.github._1c_syntax.utils.downloader with @NullMarked package-info - All log and exception messages in English Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VEHLaBE3PCoXkWraXA32Zp
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughДобавлен новый функционал загрузки BSL Language Server: класс ChangesBSL Language Server Downloader
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant BslLanguageServerDownloader
participant GitHubAPI
participant FileSystem
Caller->>BslLanguageServerDownloader: downloadIfNeeded(channel)
BslLanguageServerDownloader->>FileSystem: читать SERVER-INFO (lastUpdate)
alt интервал не истёк
BslLanguageServerDownloader-->>Caller: путь к кэшированной версии
else требуется обновление
BslLanguageServerDownloader->>GitHubAPI: получить релиз(ы) по каналу
GitHubAPI-->>BslLanguageServerDownloader: данные о релизе и ассете
BslLanguageServerDownloader->>FileSystem: скачать ZIP-ассет
BslLanguageServerDownloader->>FileSystem: распаковать ZIP (проверка path traversal)
BslLanguageServerDownloader->>FileSystem: установить POSIX-права, очистить старые версии
BslLanguageServerDownloader->>FileSystem: обновить SERVER-INFO (version, lastUpdate)
BslLanguageServerDownloader-->>Caller: путь к установленному бинарю
end
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Avoid granting group/others permissions when applying the unix mode from the downloaded archive (SonarCloud java:S2612). Owner keeps read/write and gets the execute bit when the archive marks the entry executable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VEHLaBE3PCoXkWraXA32Zp
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/test/java/com/github/_1c_syntax/utils/downloader/BslLanguageServerDownloaderTest.java (1)
46-51: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winДобавить тест для многозначных pre-release суффиксов.
Судя по реализации
compareVersionsвBslLanguageServerDownloader.java(строки 393-414), сравнение pre-release частей выполняется черезleftPre.compareTo(rightPre)— обычное лексикографическое сравнение строк. Для версий видаrc.9иrc.10это даст неверный результат ("rc.9".compareTo("rc.10") > 0, хотя семантически 10 > 9). Текущие тесты проверяют только однозначные суффиксы (rc.1vsrc.2) и не выявляют эту проблему.Стоит добавить кейс с многозначными номерами, чтобы зафиксировать ожидаемое поведение (или выявить баг в реализации).
♻️ Предлагаемое дополнение теста
`@Test` void compareVersionsTreatsReleaseAsNewerThanPreRelease() { assertThat(BslLanguageServerDownloader.compareVersions("1.0.0", "1.0.0-rc.1")).isPositive(); assertThat(BslLanguageServerDownloader.compareVersions("1.0.0-rc.1", "1.0.0")).isNegative(); assertThat(BslLanguageServerDownloader.compareVersions("1.0.0-rc.2", "1.0.0-rc.1")).isPositive(); + assertThat(BslLanguageServerDownloader.compareVersions("1.0.0-rc.10", "1.0.0-rc.9")).isPositive(); }🤖 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 `@src/test/java/com/github/_1c_syntax/utils/downloader/BslLanguageServerDownloaderTest.java` around lines 46 - 51, Add a test in BslLanguageServerDownloaderTest for multi-digit pre-release suffixes, since compareVersions currently relies on lexicographic comparison of the pre-release part in BslLanguageServerDownloader.compareVersions. Extend compareVersionsTreatsReleaseAsNewerThanPreRelease or add a new test to assert that a version like rc.10 is treated as newer than rc.9, using the existing compareVersions method as the entry point.
🤖 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
`@src/main/java/com/github/_1c_syntax/utils/downloader/BslLanguageServerDownloader.java`:
- Around line 146-178: The download flow in downloadIfNeeded should fall back to
the installed BslLanguageServer version not only when latestRelease() fails, but
also when downloadAndExtract() throws IOException during an update attempt. Wrap
the downloadAndExtract(release, latestVersion) path in the same
installed-version recovery logic used for latestRelease(), so that on any
network/I/O failure you keep the existing binaryPath(installed), call
touchLastUpdate(installed), and avoid breaking startup when a working server is
already present.
- Around line 294-306: The permissionsFromMode method is preserving group/other
permissions from the archive too loosely. Update
BslLanguageServerDownloader.permissionsFromMode so it does not copy GROUP_WRITE
or OTHERS_WRITE from the unixMode bits, while keeping the read/execute bits
behavior intact. Use the existing permissionsFromMode symbol to locate the
mapping logic and constrain the resulting PosixFilePermission set before files
are applied.
- Around line 393-414: The compareVersions method currently compares prerelease
suffixes with plain string ordering, which misorders numeric tags like rc9 and
rc10. Update the prerelease comparison in compareVersions to parse and compare
the numeric parts of identifiers in the suffix (while preserving existing
core-version and empty-suffix handling), so downloadIfNeeded makes correct
update decisions for PRERELEASE versions. Use the compareVersions and
compareCore logic as the entry point for the fix.
- Around line 194-215: Both network paths are missing request timeouts, so
downloads and GitHub API calls can block indefinitely. Update
BslLanguageServerDownloader.download() to set an HttpRequest timeout in addition
to the existing connect timeout, and update latestRelease() to build GitHub via
GitHubBuilder.withConnector(...) using a connector configured with timeouts
instead of the default client. Keep the changes localized around download() and
latestRelease() so the timeout behavior is applied consistently for both release
channels.
- Around line 194-215: In latestRelease(BslLanguageServerReleaseChannel), the
PRERELEASE branch eagerly loads all releases via listReleases().toList() and
then recomputes the newest one, which is unnecessary because GitHub already
returns newest-first. Change the selection logic to stop at the first non-draft
release from repository.listReleases() without materializing the full list,
unless you intentionally need publishedAt-based ordering; in that case keep
max(...), but otherwise simplify the stream to avoid extra GitHub requests.
---
Nitpick comments:
In
`@src/test/java/com/github/_1c_syntax/utils/downloader/BslLanguageServerDownloaderTest.java`:
- Around line 46-51: Add a test in BslLanguageServerDownloaderTest for
multi-digit pre-release suffixes, since compareVersions currently relies on
lexicographic comparison of the pre-release part in
BslLanguageServerDownloader.compareVersions. Extend
compareVersionsTreatsReleaseAsNewerThanPreRelease or add a new test to assert
that a version like rc.10 is treated as newer than rc.9, using the existing
compareVersions method as the entry point.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d985f6a2-ca0b-4a34-958a-408fd74d4fa0
📒 Files selected for processing (5)
build.gradle.ktssrc/main/java/com/github/_1c_syntax/utils/downloader/BslLanguageServerDownloader.javasrc/main/java/com/github/_1c_syntax/utils/downloader/BslLanguageServerReleaseChannel.javasrc/main/java/com/github/_1c_syntax/utils/downloader/package-info.javasrc/test/java/com/github/_1c_syntax/utils/downloader/BslLanguageServerDownloaderTest.java
| private static Set<PosixFilePermission> permissionsFromMode(int mode) { | ||
| var permissions = EnumSet.noneOf(PosixFilePermission.class); | ||
| if ((mode & 0400) != 0) permissions.add(PosixFilePermission.OWNER_READ); | ||
| if ((mode & 0200) != 0) permissions.add(PosixFilePermission.OWNER_WRITE); | ||
| if ((mode & 0100) != 0) permissions.add(PosixFilePermission.OWNER_EXECUTE); | ||
| if ((mode & 0040) != 0) permissions.add(PosixFilePermission.GROUP_READ); | ||
| if ((mode & 0020) != 0) permissions.add(PosixFilePermission.GROUP_WRITE); | ||
| if ((mode & 0010) != 0) permissions.add(PosixFilePermission.GROUP_EXECUTE); | ||
| if ((mode & 0004) != 0) permissions.add(PosixFilePermission.OTHERS_READ); | ||
| if ((mode & 0002) != 0) permissions.add(PosixFilePermission.OTHERS_WRITE); | ||
| if ((mode & 0001) != 0) permissions.add(PosixFilePermission.OTHERS_EXECUTE); | ||
| return permissions; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Права доступа "для остальных"/группы копируются из архива без ограничений.
SonarCloud отметил строки 302-304: биты GROUP_WRITE/OTHERS_WRITE/OTHERS_READ/OTHERS_EXECUTE копируются как есть из unixMode записи архива. Если артефакт релиза (или скомпрометированная точка доставки) содержит запись с правами на запись для группы/остальных, распакованные файлы (включая исполняемый бинарь сервера) окажутся доступны на запись всем пользователям системы.
🛡️ Предлагаемое исправление — не переносить биты записи для группы/остальных
if ((mode & 0040) != 0) permissions.add(PosixFilePermission.GROUP_READ);
- if ((mode & 0020) != 0) permissions.add(PosixFilePermission.GROUP_WRITE);
if ((mode & 0010) != 0) permissions.add(PosixFilePermission.GROUP_EXECUTE);
if ((mode & 0004) != 0) permissions.add(PosixFilePermission.OTHERS_READ);
- if ((mode & 0002) != 0) permissions.add(PosixFilePermission.OTHERS_WRITE);
if ((mode & 0001) != 0) permissions.add(PosixFilePermission.OTHERS_EXECUTE);Примечание: PMD-предупреждения "AvoidUsingOctalValues" на строках 296-301 — ложное срабатывание, восьмеричная запись здесь идиоматична для Unix-битов прав доступа.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static Set<PosixFilePermission> permissionsFromMode(int mode) { | |
| var permissions = EnumSet.noneOf(PosixFilePermission.class); | |
| if ((mode & 0400) != 0) permissions.add(PosixFilePermission.OWNER_READ); | |
| if ((mode & 0200) != 0) permissions.add(PosixFilePermission.OWNER_WRITE); | |
| if ((mode & 0100) != 0) permissions.add(PosixFilePermission.OWNER_EXECUTE); | |
| if ((mode & 0040) != 0) permissions.add(PosixFilePermission.GROUP_READ); | |
| if ((mode & 0020) != 0) permissions.add(PosixFilePermission.GROUP_WRITE); | |
| if ((mode & 0010) != 0) permissions.add(PosixFilePermission.GROUP_EXECUTE); | |
| if ((mode & 0004) != 0) permissions.add(PosixFilePermission.OTHERS_READ); | |
| if ((mode & 0002) != 0) permissions.add(PosixFilePermission.OTHERS_WRITE); | |
| if ((mode & 0001) != 0) permissions.add(PosixFilePermission.OTHERS_EXECUTE); | |
| return permissions; | |
| } | |
| private static Set<PosixFilePermission> permissionsFromMode(int mode) { | |
| var permissions = EnumSet.noneOf(PosixFilePermission.class); | |
| if ((mode & 0400) != 0) permissions.add(PosixFilePermission.OWNER_READ); | |
| if ((mode & 0200) != 0) permissions.add(PosixFilePermission.OWNER_WRITE); | |
| if ((mode & 0100) != 0) permissions.add(PosixFilePermission.OWNER_EXECUTE); | |
| if ((mode & 0040) != 0) permissions.add(PosixFilePermission.GROUP_READ); | |
| if ((mode & 0010) != 0) permissions.add(PosixFilePermission.GROUP_EXECUTE); | |
| if ((mode & 0004) != 0) permissions.add(PosixFilePermission.OTHERS_READ); | |
| if ((mode & 0001) != 0) permissions.add(PosixFilePermission.OTHERS_EXECUTE); | |
| return permissions; | |
| } |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 304-304: Make sure this permission is safe.
[warning] 303-303: Make sure this permission is safe.
[warning] 302-302: Make sure this permission is safe.
🪛 PMD (7.25.0)
[Medium] 296-296: AvoidUsingOctalValues (Error Prone): Avoid integer literals that start with zero (interpreted as octal), remove the leading 0 to get a decimal literal (or use explicit 0x, 0b prefixes)
(AvoidUsingOctalValues (Error Prone))
[Medium] 297-297: AvoidUsingOctalValues (Error Prone): Avoid integer literals that start with zero (interpreted as octal), remove the leading 0 to get a decimal literal (or use explicit 0x, 0b prefixes)
(AvoidUsingOctalValues (Error Prone))
[Medium] 298-298: AvoidUsingOctalValues (Error Prone): Avoid integer literals that start with zero (interpreted as octal), remove the leading 0 to get a decimal literal (or use explicit 0x, 0b prefixes)
(AvoidUsingOctalValues (Error Prone))
[Medium] 299-299: AvoidUsingOctalValues (Error Prone): Avoid integer literals that start with zero (interpreted as octal), remove the leading 0 to get a decimal literal (or use explicit 0x, 0b prefixes)
(AvoidUsingOctalValues (Error Prone))
[Medium] 300-300: AvoidUsingOctalValues (Error Prone): Avoid integer literals that start with zero (interpreted as octal), remove the leading 0 to get a decimal literal (or use explicit 0x, 0b prefixes)
(AvoidUsingOctalValues (Error Prone))
[Medium] 301-301: AvoidUsingOctalValues (Error Prone): Avoid integer literals that start with zero (interpreted as octal), remove the leading 0 to get a decimal literal (or use explicit 0x, 0b prefixes)
(AvoidUsingOctalValues (Error Prone))
🤖 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
`@src/main/java/com/github/_1c_syntax/utils/downloader/BslLanguageServerDownloader.java`
around lines 294 - 306, The permissionsFromMode method is preserving group/other
permissions from the archive too loosely. Update
BslLanguageServerDownloader.permissionsFromMode so it does not copy GROUP_WRITE
or OTHERS_WRITE from the unixMode bits, while keeping the read/execute bits
behavior intact. Use the existing permissionsFromMode symbol to locate the
mapping logic and constrain the resulting PosixFilePermission set before files
are applied.
Source: Linters/SAST tools
- Fall back to the installed server when the download/extract step fails mid-update, matching the offline-recovery contract (only latestRelease failures were handled before) - Add request timeout to the asset download and a connect timeout to the GitHub API client (HttpClientGitHubConnector) - Compare pre-release identifiers numerically (rc.10 > rc.9) instead of lexicographically, so PRERELEASE update decisions are correct - Pick the first non-draft release instead of materializing and re-sorting - Add a multi-digit pre-release comparison test Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VEHLaBE3PCoXkWraXA32Zp
Replace the hand-rolled version/pre-release comparison with the org.semver4j:semver4j library, which implements semver ordering (numeric pre-release identifiers, release > pre-release) correctly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VEHLaBE3PCoXkWraXA32Zp
|
2 participants
Что это
Загрузчик BSL Language Server из GitHub-релизов — общий компонент для клиентов (в первую очередь для плагина
intellij-language-1c-bsl, переезжающего на LSP4IJ). Реализация повторяет поведение загрузчика из vsc-language-1c-bsl, чтобы раскладка каталога установки и правила выбора ассета совпадали между VS Code и IntelliJ-клиентами.Развивает идею давнего PR #3, но в актуальном виде: полный цикл (скачивание → распаковка → поиск исполняемого файла), без обязательного токена, современные зависимости.
Изменения
downloader/BslLanguageServerDownloader— определяет ОС (win/mac/nix), скачивает ассетbsl-language-server_{os}.zipпоследнего релиза или pre-release, кэширует установку по версии (файлSERVER-INFO+ троттлинг обращений к GitHub API), распаковывает архив с сохранением unix-прав (важно для native-image launcher и встроенного JRE) и возвращает путь к исполняемому файлу сервера.downloader/BslLanguageServerReleaseChannel— канал релизов (STABLE/PRERELEASE).downloader/package-info.java—@NullMarked(jspecify).Зависимости
org.kohsuke:github-api— список релизов/ассетов;org.apache.commons:commons-compress— распаковка zip с сохранением POSIX-прав;org.slf4j:slf4j-api— логирование.Скачивание бинарных файлов идёт через встроенный
java.net.http.HttpClient(без токена).Проверки
./gradlew test)../gradlew build(включаяlicenseMain/javadoc) — зелёный.🤖 Generated with Claude Code
Generated by Claude Code