add: возможность копировать интегралки текстом#8483
add: возможность копировать интегралки текстом#8483kkadmor wants to merge 22 commits intoss220-space:master220from
Conversation
There was a problem hiding this comment.
Code Review
Аналитический отчёт от Большого Брата. Обработка запроса завершена.
Приветствую! Я проанализировал предложенные Вами изменения. С технической точки зрения, работа выглядит требующей доработок.
Сводка анализа:
- 🔴 Критических замечаний: 2
- 🟡 Рекомендаций: 7
- 💡 Предложений: 0
[При наличии] 🔴 Критические замечания (требуют обязательного исправления):
code/modules/wiremod/core/hmac.dm:20: Некорректное дополнение ключа в реализации HMAC, что приводит к неверной подписи.code/modules/research/circuitprinter.dm:465: Ошибка в логике проверки импортируемой строки, которая будет отклонять все корректные платы.
[При наличии] 🟡 Рекомендации по улучшению:
code/__HELPERS/text.dm:65: Комментарий в коде должен быть на английском языке согласно стайлгайду.code/modules/research/circuitprinter.dm:312: Комментарий в коде должен быть на английском языке согласно стайлгайду.code/modules/research/circuitprinter.dm:324-343: Повторяющийся код проверки JSON-полей можно улучшить для повышения читаемости и упрощения поддержки.code/modules/research/circuitprinter.dm:347: Комментарий в коде должен быть на английском языке согласно стайлгайду.code/modules/research/circuitprinter.dm:348-352: Избыточный вызовjson_decodeвsave_circuit_by_json, что снижает производительность.code/modules/research/circuitprinter.dm:442: Комментарий в коде должен быть на английском языке согласно стайлгайду.code/modules/wiremod/core/hmac.dm:90-92: Неочевидная логика валидации длины ключа HMAC.
Пожалуйста, ознакомьтесь с комментариями к отдельным строкам кода для получения более подробной информации.
Спасибо за ваш вклад в проект!
|
|
||
| var/hmac_key = CONFIG_GET(string/hmac_key) ? CONFIG_GET(string/hmac_key) : "" | ||
|
|
||
| if(!text || findtext(text, ".")) |
There was a problem hiding this comment.
🔴 Критическая ошибка: Условие if(!text || findtext(text, ".")) некорректно. Оно приводит к тому, что любая корректная строка для импорта (которая должна содержать точку в качестве разделителя) будет отклонена. findtext возвращает позицию, и если точка найдена, условие становится истинным, что вызывает ошибку. Условие следует инвертировать, чтобы проверять отсутствие точки.
if(!text || !findtext(text, "."))
code/modules/wiremod/core/hmac.dm
Outdated
| key_bin = hex_to_bin(md5(key_bin)) | ||
|
|
||
| if(length(key_bin) < HMAC_BLOCK_SIZE) | ||
| key_bin += repeat_string(HMAC_BLOCK_SIZE - length(key_bin), "0") |
There was a problem hiding this comment.
🔴 Критическая ошибка безопасности: Ключ HMAC дополняется символом '0' (ascii2text(48)) вместо нулевого байта (ascii2text(0)). Согласно стандарту HMAC, дополнение должно производиться нулевыми байтами. Эта ошибка приводит к генерации неверной HMAC-подписи, что полностью нарушает механизм проверки целостности данных.
key_bin += repeat_string(HMAC_BLOCK_SIZE - length(key_bin), ascii2text(0))
There was a problem hiding this comment.
Нулевой байт не добавляет ничего к строке и она никак не меняется. Поэтому "0"
| for(var/i in 1 to times) | ||
| . += string | ||
|
|
||
| // Создает строку используя список из ASCII |
There was a problem hiding this comment.
🟡 Рекомендация по стилю: Комментарий в коде должен быть на английском языке. Это требование стайлгайда (пункт 4.1, строка 68) для обеспечения согласованности кодовой базы.
// Creates a string using a list of ASCII codes
References
- Комментарии в коде должны быть на английском. (link)
|
|
||
| update_static_data_for_all_viewers() | ||
|
|
||
| // Проверяет наличие всех данных |
There was a problem hiding this comment.
🟡 Рекомендация по стилю: Комментарий в коде должен быть на английском языке. Это требование стайлгайда (пункт 4.1, строка 68) для обеспечения согласованности кодовой базы.
// Checks for presence of all data
References
- Комментарии в коде должны быть на английском. (link)
| if(!LAZYACCESS(data, "dupe_data")) | ||
| return FALSE | ||
|
|
||
| if(!LAZYACCESS(data, "name")) | ||
| return FALSE | ||
|
|
||
| if(!LAZYACCESS(data, "materials")) | ||
| return FALSE | ||
|
|
||
| if(!LAZYACCESS(data, "integrated_circuit")) | ||
| return FALSE | ||
|
|
||
| if(!LAZYACCESS(data, "Icon")) | ||
| return FALSE | ||
|
|
||
| if(!LAZYACCESS(data, "IconState")) | ||
| return FALSE | ||
|
|
||
| if(!LAZYACCESS(data, "desc")) | ||
| return FALSE |
There was a problem hiding this comment.
🟡 Рекомендация по улучшению: Блок проверки полей JSON содержит много однотипных проверок. Для улучшения читаемости и упрощения дальнейшей поддержки кода, этот блок можно отрефакторить, используя цикл по списку обязательных ключей.
var/list/required_keys = list("dupe_data", "name", "materials", "integrated_circuit", "Icon", "IconState", "desc")
for(var/key in required_keys)
if(!LAZYACCESS(data, key))
return FALSE
|
|
||
| return TRUE | ||
|
|
||
| // Для иморта платы |
There was a problem hiding this comment.
🟡 Рекомендация по стилю: Комментарий в коде должен быть на английском языке. Это требование стайлгайда (пункт 4.1, строка 68) для обеспечения согласованности кодовой базы.
// For board import
References
- Комментарии в коде должны быть на английском. (link)
| /obj/machinery/r_n_d/circuit_imprinter/proc/save_circuit_by_json(mob/living/user, json_data) | ||
| if(!can_save_circuit_by_json(user, json_data)) | ||
| return | ||
|
|
||
| var/list/data = json_decode(json_data) |
There was a problem hiding this comment.
🟡 Рекомендация по производительности: Процедура save_circuit_by_json вызывает can_save_circuit_by_json, которая уже выполняет json_decode. Затем save_circuit_by_json выполняет json_decode повторно. Это избыточно и немного снижает производительность. Рекомендуется передавать уже декодированные данные из can_save_circuit_by_json или изменить логику, чтобы избежать двойного декодирования.
|
|
||
| var/hmac_key = CONFIG_GET(string/hmac_key) ? CONFIG_GET(string/hmac_key) : "" | ||
|
|
||
| // Создает HMAC если ключ корректен |
There was a problem hiding this comment.
🟡 Рекомендация по стилю: Комментарий в коде должен быть на английском языке. Это требование стайлгайда (пункт 4.1, строка 68) для обеспечения согласованности кодовой базы.
// Creates HMAC if the key is correct
References
- Комментарии в коде должны быть на английском. (link)
code/modules/wiremod/core/hmac.dm
Outdated
| var/len = length(hex_key) | ||
| if(len != 40 && len != 64) | ||
| return FALSE |
There was a problem hiding this comment.
🟡 Рекомендация по улучшению: Функция validate_hmac_key проверяет длину ключа на равенство 40 или 64 символам. В описании пулл-реквеста и в файле конфигурации упоминается только длина в 40 символов. Причина для поддержки 64-символьной длины не ясна. Рекомендуется либо добавить комментарий, объясняющий эту логику, либо убрать проверку на 64 символа для соответствия документации.
|
/gemini review |
There was a problem hiding this comment.
Code Review
Аналитический отчёт от Большого Брата. Обработка запроса завершена.
Приветствую! Я проанализировал предложенные Вами изменения. С технической точки зрения, работа выглядит требующей доработок.
Сводка анализа:
- 🔴 Критических замечаний: 1
- 🟡 Рекомендаций: 6
- 💡 Предложений: 0
🔴 Критические замечания (требуют обязательного исправления):
code/modules/wiremod/core/hmac.dm:19-20: Обнаружена критическая уязвимость в реализации HMAC. Ключ дополняется некорректно, что может привести к возможности подделки данных плат.
🟡 Рекомендации по улучшению:
code/modules/research/circuitprinter.dm:313-319: Процедураcan_save_circuit_by_jsonне обрабатывает некорректный JSON, что может привести к ошибке выполнения.code/modules/research/circuitprinter.dm:440: В логике импорта обнаружен повторяющийся вызов функции, что снижает производительность и читаемость кода.code/modules/research/circuitprinter.dm:450: Аналогично, обнаружен повторяющийся вызов функции.code/__HELPERS/text.dm:65: Комментарий в коде должен быть на английском языке в соответствии со стайлгайдом проекта.code/modules/research/circuitprinter.dm:312: Комментарии в коде должны быть на английском языке.code/modules/wiremod/core/hmac.dm:5-9: Комментарии в коде должны быть на английском языке.
Пожалуйста, ознакомьтесь с комментариями к отдельным строкам кода для получения более подробной информации.
Спасибо за ваш вклад в проект!
| if(length(key_bin) < HMAC_BLOCK_SIZE) | ||
| key_bin += repeat_string(HMAC_BLOCK_SIZE - length(key_bin), "0") |
There was a problem hiding this comment.
🔴 Критично: Некорректная реализация HMAC. Ключ дополняется ASCII-символами '0' ("0") вместо нулевых байтов. Согласно RFC 2104, ключ, который короче размера блока, должен быть дополнен до размера блока нулевыми байтами. Текущая реализация использует символ "0", что приводит к неверной генерации подписи и создаёт уязвимость, позволяющую подделку данных платы.
if(length(key_bin) < HMAC_BLOCK_SIZE)
key_bin += repeat_string(HMAC_BLOCK_SIZE - length(key_bin), ascii2text(0))
| /obj/machinery/r_n_d/circuit_imprinter/proc/can_save_circuit_by_json(mob/living/user, json_data) | ||
| var/list/data = json_decode(json_data) | ||
| var/list/required_keys = list("dupe_data", "name", "materials", "integrated_circuit", "Icon", "IconState", "desc") | ||
| for(var/key in required_keys) | ||
| if(!LAZYACCESS(data, key)) | ||
| return FALSE | ||
| return TRUE |
There was a problem hiding this comment.
🟡 Рекомендация: Обнаружена потенциальная ошибка выполнения в процедуре can_save_circuit_by_json. Функция json_decode() может вернуть null, если json_data содержит некорректный JSON. В этом случае последующее обращение к data в цикле for вызовет ошибку. Рекомендую добавить проверку if(!islist(data)) return FALSE после вызова json_decode() для предотвращения падения сервера.
/obj/machinery/r_n_d/circuit_imprinter/proc/can_save_circuit_by_json(mob/living/user, json_data)
var/list/data = json_decode(json_data)
if(!islist(data))
return FALSE
var/list/required_keys = list("dupe_data", "name", "materials", "integrated_circuit", "Icon", "IconState", "desc")
for(var/key in required_keys)
if(!LAZYACCESS(data, key))
return FALSE
return TRUE
References
- Требовать проверку на
null(if(!variable))перед использованием объекта, особенно послеlocate(),find_in_view()и т.п. (link)
| for(var/i in 1 to times) | ||
| . += string | ||
|
|
||
| // Создает строку используя список из ASCII |
There was a problem hiding this comment.
🟡 Рекомендация по стилю: Согласно стайлгайду (пункт 4.1, правило о комментариях), все комментарии в коде должны быть на английском языке.
// Creates a string using a list of ASCII codes
References
- Комментарии должны быть на английском языке и объяснять 'почему', а не 'что'. (link)
|
|
||
| update_static_data_for_all_viewers() | ||
|
|
||
| // Проверяет наличие всех данных |
There was a problem hiding this comment.
🟡 Рекомендация по стилю: Согласно стайлгайду (пункт 4.1, правило о комментариях), все комментарии в коде должны быть на английском языке. В этом файле несколько таких комментариев.
// Checks if all data is present
References
- Комментарии должны быть на английском языке и объяснять 'почему', а не 'что'. (link)
| var/mob/user = ui.user | ||
|
|
||
| var/text = tgui_input_text(user, "Вставьте текст интегральной платы", "Импорт схемы", encode=FALSE, max_length = 999999999) | ||
| text = text && ascii_list2text(rustlib_decode_base64(text)) ? ascii_list2text(rustlib_decode_base64(text)) : "" |
There was a problem hiding this comment.
|
|
||
| var/list/parts = splittext(text, ".") // [1] - json в виде списка ASCII [2] - HMAC в виде списка ASCII | ||
|
|
||
| var/json_data = ascii_list2text(rustlib_decode_base64(parts[1])) ? ascii_list2text(rustlib_decode_base64(parts[1])) : "" |
There was a problem hiding this comment.
🟡 Рекомендация по улучшению: Аналогично предыдущему замечанию, вызов ascii_list2text(rustlib_decode_base64(parts[1])) повторяется. Рекомендуется сохранить результат в переменную.
var/list/decoded_json_b64 = rustlib_decode_base64(parts[1])
var/json_data = decoded_json_b64 ? ascii_list2text(decoded_json_b64) : ""
| /* | ||
| * Основная функция HMAC-MD5 | ||
| * hex_key - ключ в hex-формате | ||
| * data - данные для подписи | ||
| */ |
There was a problem hiding this comment.
🟡 Рекомендация по стилю: Согласно стайлгайду (пункт 4.1, правило о комментариях), все комментарии в коде должны быть на английском языке.
/*
* Main HMAC-MD5 function
* hex_key - key in hex format
* data - data to be signed
*/
References
- Комментарии должны быть на английском языке и объяснять 'почему', а не 'что'. (link)
|
Ожидает ревью и тд. |
Что этот ПР делает
Копировать плату
В принтере плат рядом с каждой сохраненной платой появилась кнопка "Export". При нажатии появляется окно для ввода, со вставленным текстом платы. Текст платы - закодированные в таком виде данные: "[json_data].[hmac_base64]". Для уверенности в том, что плату не подделали (не поменяли определенные параметры) используется электронная подпись HMAC SHA-256
Импортировать плату с помощью полученного текста
В принтере плат сверху появилась кнопка "Import". При нажатии появляется окно ввода, куда пользователь пишет заранее полученный код. После чего код расшифровывается и проверяется (проверка на правильность данных, сравнение переданного и полученного HMAC)
В конфиг (config/config.txt) добавленна переменная HMAC_KEY которая хранит HMAC ключ в виде HEX в строке. По умолчанию "ffffffffffffffffffffffffffffffffffffffff", при деплое необходимо поменять на любой HEX из 40 символов и не менять (изменение ключа приведет к тому, что ранее скопированные платы перестанут импортироваться)
Почему это хорошо для игры
Неудобно каждый раз заново делать платы, которые уже придуманы. Позволяет игрокам делать более сложные и интересные платы
Демонстрация изменений
Демонстрации изменений
Тестирование
На локалке. Пробовал менять случайные символы полученного кода.