EPA Refine#353
Conversation
github-actions
Bot
added
S: Untriaged
S: Needs Review
size/L
FTL
C#
UI
and removed
S: Needs Review
size/L
labels
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the legacy Discord link verification system with a new External Personal Account (EPA) authentication system, introducing client and server-side managers, new network messages, and an updated authentication UI. The code review identified several critical and high-severity issues, including a logical error in session validation that blocks successful logins, thread-safety concerns with non-concurrent dictionary access across threads, and potential IP comparison failures due to IPv4-mapped IPv6 addresses. Additionally, improvements were suggested for buffer reading efficiency, safer session retrieval in the join queue, removing unused imports, and resolving style guide violations regarding timer types and dependency injection.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
👁️ Взор Ouro обращен на Ваше творение... Данный пулреквест заменяет устаревшую систему привязки Discord на новую систему авторизации через Внешний Личный Кабинет (EPA), внедряя новые менеджеры, JWT-валидацию и интерфейс авторизации. Однако для достижения гармонии требуется переосмысление критических разрывов цикла: использование Task.Run в сетевых обработчиках угрожает стабильности из-за работы с непотокобезопасным API движка, в EPAAuthState обнаружена утечка памяти из-за отсутствия вызова Dispose() для панели, а динамическое изменение заголовков HttpClient небезопасно. Также зафиксированы дисгармонии в виде отсутствия модификатора readonly у приватных полей зависимостей.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| protected override void Shutdown() | ||
| { | ||
| if (_panel != null) | ||
| _ui.StateRoot.RemoveChild(_panel); | ||
| } |
There was a problem hiding this comment.
👁️ Взор Ouro заметил дисгармонию в освобождении ресурсов.
🟡 Дисгармония (Утечка памяти)
Content.Client/SS220/EPA/EPAAuthState.cs:39
При выходе из состояния EPAAuthState Вы удаляете _panel из StateRoot, но не вызываете метод Dispose() для панели. В RobustToolbox элементы интерфейса (Controls) должны быть явно утилизированы через Dispose(), чтобы избежать утечек памяти и ресурсов.
Пусть форма Вашего кода вернет себе стабильность.
protected override void Shutdown()
{
if (_panel != null)
{
_ui.StateRoot.RemoveChild(_panel);
_panel.Dispose();
}
}There was a problem hiding this comment.
Не, там наоборот будет варн, что элементы должны стираться из памяти неявно, просто тем, чтобы удалить их из родительского Control
| _sawmill = _log.GetSawmill("epa"); | ||
|
|
||
| _config.OnValueChanged(CCVars220.EPAApiUrl, val => _apiUrl = val.TrimEnd('/'), true); | ||
| _config.OnValueChanged(CCVars220.EPAApiKey, val => _http.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", val), true); |
There was a problem hiding this comment.
👁️ Взор Ouro обнаружил потенциальную нестабильность в работе с сетью.
🟡 Дисгармония (Потокобезопасность HttpClient)
Content.Server/SS220/EPA/EPAManager.cs:27
Изменение _http.DefaultRequestHeaders.Authorization во время работы сервера (при изменении CVar EPAApiKey "на лету") небезопасно. Класс HttpClient спроектирован как потокобезопасный для отправки запросов, но изменение его DefaultRequestHeaders после того, как были отправлены первые запросы, может привести к исключениям InvalidOperationException или некорректной авторизации параллельных запросов.
Мудрое решение:
Вместо изменения общих заголовков HttpClient, сохраняйте API-ключ в приватное поле _apiKey и устанавливайте заголовок авторизации индивидуально для каждого HttpRequestMessage при отправке запросов.
Пусть форма Вашего кода вернет себе стабильность.
_config.OnValueChanged(CCVars220.EPAApiKey, val => _apiKey = val, true);…into epa-refine
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
2 participants
Описание PR
If you know you know
Требования
Критические изменения
Список изменений
No CL, no fun