Мы используем куки, чтобы пользоваться сайтом
было удобно.
2025 год подходит к концу. Minecraft моды, каталонский язык и неочевидные взаимодействия с тернарным оператором — с чем только не успел познакомиться наш анализатор. А значит, самое время вам об этом рассказать — представляем топ-10 ошибок, которые нашёл анализатор PVS-Studio в open source проектах за 2025 год.
В течение этого года мы проверяли open source проекты и писали статьи о результатах этих проверок.
В конце года из всех найденных ошибок и необычных моментов мы отобрали те, которые, по нашему субъективному мнению, оказались наиболее интересными. И эта статья — как раз та самая подборка. Мы отобрали 10 ошибок, о которых, в порядке возрастания субъективной крутости, расскажем далее.
Приступаем.
Открывает наш топ ошибка из проекта languagetool. В каком-то смысле он нам особенно близок, поскольку одна из его задач тоже состоит в анализе языков. Разве что, в отличие от нас, работает он естественными языками, а не с искусственными.
Главный нюанс состоит в том, что это ошибка нашего анализатора, но она показалась нам настолько забавной, что не рассказать о ней я не могу. И к тому же, ну какой Новый Год без контркультурной выходки?
Сам фрагмент кода, в котором анализатор выдал срабатывание:
public String getEnclitic(AnalyzedToken token) {
....
if (word.endsWith("ه")) {
suffix = "ه";
....
else if ((word.equals("عني") ||
word.equals("مني")) &&
word.endsWith("ني") // <=
) {
suffix = "ني";
}
....
}Предупреждение PVS-Studio: V6007 Expression 'word.endsWith("ني")' is always false. ArabicTagger.java 428
Если смотреть на строковые константы так, как привыкли носители английского, французского, немецкого или русского языка — сообщение анализатора действительно кажется справедливым. Однако здесь стоит помнить о том, что в арабском языке написание и чтение происходит справа-налево. А значит, там, где не носитель арабского языка ожидает конец литерала, на самом деле он столкнётся с его началом. И в этой ситуации анализатор оказался не носителем арабского языка.
К слову, если учесть RTL (right-to-left, справа-налево) написание слова, окажется, что условие на самом деле всегда истинно, а не ложно.
Мне стало интересно: "А как Java обрабатывает языки с письмом справа налево?" Секрет в том, что Java работает не с "визуальным" представлением текста, а с его Unicode-кодировкой. В Unicode символы любых языков — как с направлением слева направо, так и справа налево — хранятся в логическом порядке, то есть в том порядке, в котором они вводятся и читаются носителями языка.
Это означает, что Java не требуется выполнять какие-либо специальные преобразования для работы с RTL-текстом. Строка остаётся обычной последовательностью Unicode-точек, а такие операции, как получение длины строки, доступ к символам или выделение подстроки, работают одинаково для всех языков.
Поэтому для вот такой проверки:
System.out.println("مرحباً بالجميع".endsWith("بالجميع"));Результатом будет true, хотя если читать слева направо, то строка начинается, а не закачивается с "بالجميع".
Для нас ошибка оказалась познавательной и интересной, но поскольку всё же мы здесь неправы – 10 место. С самой статьёй о проверке этого проекта можно ознакомиться здесь.
И снова ошибка из languagetool.
В модуле каталонского языка есть метод removeOldDiacritics(), который аккуратно правит устаревшие орфографические формы и лишние диакритики — превращает adéu в adeu, dóna в dona, vénen в venen и т.д:
private String removeOldDiacritics(String s) {
return s
.replace("contrapèl", "contrapel")
.replace("Contrapèl", "Contrapel")
.replace("vés", "ves")
.replace("féu", "feu")
.replace("desféu", "desfeu")
.replace("adéu", "adeu")
.replace("dóna", "dona")
.replace("dónes", "dones")
.replace("sóc", "soc")
.replace("vénen", "venen")
.replace("véns", "véns") // <=
.replace("fóra", "fora")
.replace("Vés", "Ves")
.replace("Féu", "Feu")
.replace("Desféu", "Desfeu")
.replace("Adéu", "Adeu")
.replace("Dóna", "Dona")
.replace("Dónes", "Dones")
.replace("Sóc", "Soc")
.replace("Vénen", "Venen")
.replace("Véns", "Vens")
.replace("Fóra", "Fora");
}Казалось бы, обычный метод: пришёл, поправил, никому не мешает.
Но анализатор PVS-Studio имеет другое мнение: V6009 Function 'replace' receives an odd argument. The '" véns "' argument was passed several times. Catalan.java 453
В указанном фрагменте кода закралась ошибка: слово véns мы заменяем на него же. Вероятнее всего, разработчик копировал исходное слово и заменял символ, но в этом конкретном случае забыл поменять символ во втором аргументе:
....
.replace("véns", "vens")
....За то, что анализатор искусственного языка нашёл ошибку в анализаторе естественного языка, мы добавляем это срабатывание в наш топ.
Идём далее. Речь пойдёт об ошибке из Elasticsearch.
Фрагмент исходного кода:
@Override
public boolean equals(Object obj) {
....
KeyedFilter other = (KeyedFilter) obj;
return Objects.equals(keys, other.keys)
&& Objects.equals(timestamp, other.timestamp)
&& Objects.equals(tiebreaker, other.tiebreaker)
&& Objects.equals(child(), other.child())
&& isMissingEventFilter == isMissingEventFilter; // <=
}Предупреждение PVS-Studio: V6001 There are identical sub-expressions 'isMissingEventFilter' to the left and to the right of the '==' operator. KeyedFilter.java 116
Как и в предыдущем примере, мы имеем дело с опечаткой, но есть небольшой нюанс. Как вы можете заметить, ошибка допущена в последней строке блока однотипного кода.
Подобного рода ошибки при разработке случаются достаточно часто. Настолько, что этому "феномену" придумали отдельное название: "эффект последней строки". Даже в рамках этого проекта такая ошибка не единственная: аналогичная ошибка также допущена в последней строке блока однотипного кода:
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
IndexError that = (IndexError) o;
return indexName.equals(that.indexName)
&& Arrays.equals(shardIds, that.shardIds)
&& errorType == that.errorType
&& message.equals(that.message)
&& stallTimeSeconds == stallTimeSeconds; // <=
}Предупреждение PVS-Studio: V6001 There are identical sub-expressions 'stallTimeSeconds' to the left and to the right of the '==' operator. IndexError.java 147
У нас на эту тему есть как минимум следующие статьи:
За факт принадлежности этой ошибки к такому интересному "явлению" мы награждаем её местом в нашем топе.
Переходим к седьмой позиции.
Мы рассмотрели уже три Java ошибки, а с Minecraft-ом не столкнулись ни разу. Пора бы это исправить.
Фрагмент кода из проекта CustomNPC+:
private String getTimePast() {
....
if(selected.timePast > 3600000){
int hours = (int) (selected.timePast / 3600000);
if(hours == 1)
return hours + " " +
StatCollector.translateToLocal("mailbox.hour");
else
return hours + " " +
StatCollector.translateToLocal("mailbox.hours");
}
int minutes = (int) (selected.timePast / 60000);
if(minutes == 1)
return minutes + " " +
StatCollector.translateToLocal("mailbox.minutes");
else
return minutes + " " +
StatCollector.translateToLocal("mailbox.minutes");
}Предупреждение PVS-Studio: V6004 The 'then' statement is equivalent to the 'else' statement. GuiMailbox.java 76
Здесь мы столкнулись с опечаткой, которая привела к ошибке локализации. В зависимости от количества минут должно было вывестись либо "minute", либо "minutes". Но в последнем условии у нас что в then-, что в else-ветке буквально одинаковый код, который выводит "minutes" всегда.
Как говорит сам автор статьи-проверки этого проекта: "Учитывая, что перевод для mailbox.minute уже присутствует в ресурсах игры, отправка коммита заняла больше времени, чем правка в код". И действительно, исправление заключается в том, чтобы стереть один символ. И тогда для всех поддерживаемых модом языков мы будет иметь дело с правильной формой слова.
Безусловно, ошибка не такая серьёзная, но за то, что её можно без каких-либо проблем воспроизвести, мы награждаем её седьмым местом в топе:
Двигаемся далее. На очереди у нас ошибка из проекта AutoMQ:
public static final int MAX_TYPE_NUMBER = 20;
private static final LongAdder[] USAGE_STATS = new LongAdder[MAX_TYPE_NUMBER];
....
public static ByteBuf byteBuffer(int initCapacity, int type) {
try {
if (MEMORY_USAGE_DETECT) {
....
if (type > MAX_TYPE_NUMBER) {
counter = UNKNOWN_USAGE_STATS;
} else {
counter = USAGE_STATS[type]; // <=
....
}
....
}
....
}
}Предупреждение PVS-Studio: V6025. Possibly index 'type' is out of bounds. ByteBufAlloc.java 151
Ошибка довольно распространённая (по крайней мере автор этого топа сам допускал её очень часто). Мы имеем дело с массивом размера MAX_TYPE_NUMBER. Из-за небольшой ошибки в условии, на той строке кода, на которую указывает анализатор, может случиться OutOfBoundsException.
А произойти он может из-за условия: if (type > MAX_TYPE_NUMBER). Оно допускает ситуацию, при которой в блоке else у параметра type будет значение MAX_TYPE_NUMBER, что на единицу превышает максимально допустимый индекс для массива. В случае равенства будет выброшено исключение о выходе за границу массива, поскольку в первой же строке мы обращаемся к массиву по индексу type.
Исправление, так же, как и в предыдущем примере, донельзя простое: >= вместо >.
За субъективную близость автору статьи ошибка занимает шестую позицию топа.
Пятая позиция на очереди, проект jetty. Сразу переходим к коду:
public class HelloSessionServlet extends HttpServlet {
....
@Override
protected void doGet(
HttpServletRequest request,
HttpServletResponse response
) {
....
String greeting = request.getParameter("greeting"); // <=
if (greeting != null)
{
....
message = "New greeting '" + greeting + "' set in session."; // <=
....
}
....
PrintWriter out = response.getWriter();
out.println("<h1>" + greeting + " from HelloSessionServlet</h1>"); // <=
out.println("<p>" + message + "</p>"); // <=
....
}
}Срабатывание анализатора PVS-Studio: V5330. Possible XSS injection. Potentially tainted data in the 'message' variable might be used to execute a malicious script. HelloSessionServlet.java 70
Анализатор говорит о том, что здесь возможна XSS-инъекция. Давайте посмотрим.
В коде мы получаем строку из запроса, на основе неё формируем сообщения, а потом на основе этих сообщений формируем html-страницу. Поскольку данные из запроса никак не санитизируются, здесь действительно возможен XSS.
Если переданный извне message будет представлять собой, к примеру, строку <script>alert("XSS Injection")</script>, то Java-script код alert("XSS Injection") выполнится при открытии страницы. Как и любой другой js код.
Но если речь идёт о реальной уязвимости, почему это не первое место топа? Всё дело в том, что мы имеем дело с демонстрационным примером. То есть этот код нужен сугубо для ознакомления с возможностями jetty. И тем не менее, стоит помнить о том, что демонстрационный пример не исключает фактор риска, поскольку он может стать источником заимствования. Всё же "демонстрировать" небезопасный код — не лучшая практика.
За первое taint-срабатывание в реальном проекте, хоть и в демонстрационном примере, мы награждаем его 5-ым местом в топе.
Ошибка из проекта Elasticsearch.
Фрагмент кода с ошибкой:
public static Number truncate(Number n, Number precision) {
....
Double result = (((n.doubleValue() < 0)
? Math.ceil(g)
: Math.floor(g)) / tenAtScale);
return n instanceof Float ? result.floatValue() : result; // <=
}Срабатывание анализатора PVS-Studio: V6088 Result of this expression will be implicitly cast to 'double'. Check if program logic handles it correctly. Maths.java 122
Этот метод "усекает" переданное число в зависимости от заданной точности. Для целых чисел — сокращает заданное количество разрядов, а для дробных — убирает лишние числа после запятой. Выше приведён фрагмент, где работа ведётся с дробным числом.
Судя по тернарному оператору, планировалось, что в случае с дробными числами метод вернёт число того же типа, с котором оно пришло в метод (либо Float, либо Double). Но не самый очевидный нюанс здесь сыграл свою роль.
Сам по себе тернарный оператор является выражением. Как следствие, возвращаемый им результат должен иметь один заранее определённый тип. И тут вступают в игру правила из Numeric Context. Если кратко, то нам здесь важно следующее:
Double, поскольку он в себя вмещает и Double, и Float.То есть, несмотря на ожидания, в случае с дробными числами метод всегда будет возвращать Double значение.
Компактная форма записи действительно подвела. Возможный вариант исправления выглядит так:
public static Number truncate(Number n, Number precision) {
....
if (n instanceof Float) {
return result.floatValue();
} else {
return result;
}
}За неочевидный языковой нюанс ошибка награждается четвёртым местом в топе.
И мы переходим к подиуму. Бронзовый призёр располагается в проекте OpenIDE. Итак, сам код:
public final class UsageType {
....
public static final UsageType
DELEGATE_TO_ANOTHER_INSTANCE_PARAMETERS_CHANGED = new UsageType(
UsageViewBundle.messagePointer(
"usage.type.delegate.to.another.instance.method.parameters.changed"
)
);
private static final Logger LOG = Logger.getInstance(UsageType.class);
public UsageType(@NotNull Supplier<....> nameComputable) {
myNameComputable = nameComputable;
if (ApplicationManager.getApplication().isUnitTestMode()) {
String usageTypeString = myNameComputable.get();
if (usageTypeString.indexOf('{') != -1) {
LOG.error(....);
}
}
}
....
}Срабатывание анализатора PVS-Studio: V6050 Class initialization cycle is present. Initialization of 'DELEGATE_TO_ANOTHER_INSTANCE_PARAMETERS_CHANGED' appears before the initialization of 'LOG'. UsageType.java 46
Анализатор говорит, что мы имеем дело с циклической зависимостью при инициализации статических полей. Снова не самый очевидный момент, связанный с нюансами языка Java.
Для инициализации поля DELEGATE_TO_ANOTHER_INSTANCE_PARAMETERS_CHANGED используется конструктор. А в нём мы обращаемся к логгеру LOG.
Всё дело в том, что поля инициализируются в порядке их объявления. Как вы можете видеть, DELEGATE_TO_ANOTHER_INSTANCE_PARAMETERS_CHANGED располагается выше LOG, т.е. инициализируется раньше. Как следствие, LOG на момент исполнения конструктора будет равен null, и при обращении к нему мы столкнёмся с NPE.
Исправление, как и в большинстве случаев выше, будет донельзя простым. Нужно просто разместить объявление LOG выше, чем DELEGATE_TO_ANOTHER_INSTANCE_PARAMETERS_CHANGED.
Мы перешли к серебренному призёру. И снова с нами Minecraft мод CustomNPC+.
Фрагмент кода, в котором допущена ошибка:
public int range = 5;
public int speed = 5;
....
public boolean aiShouldExecute() {
healTicks++;
if (healTicks < speed * 10)
return false;
for (Object plObj : npc.worldObj.getEntitiesWithinAABB(
EntityLivingBase.class,
npc.boundingBox.expand(
range,
range / 2, // <=
range))
) {
....
}
healTicks = 0;
return !toHeal.isEmpty();
}Срабатывание анализатора PVS-Studio: V6094 The expression was implicitly cast from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. JobHealer.java 41
В этом фрагменте кода с промежутком времени, указанным в поле speed, выполняется поиск всех сущностей в прямоугольнике с радиусом range вокруг NPC с профессией лекаря.
Поскольку у метода expand сигнатура expand(double x, double y, double z), срабатывание анализатора здесь по делу: методу передаётся int, когда ожидается double. "Ну и что с того?" — спросите вы. Нам это показалось странным, и мы решили разобраться.
Такой небольшой недочёт приводит к тому, что если радиус — нечётное число, при расчёте координаты Y мы теряем по половине блока сверху и снизу (то есть суммарно целый блок). Убедиться в этом нам помогла визуализация коллизии, которая рассчитывается на основе радиуса. Поделимся этой визуализацией с вами:
И да, как вы можете заметить, если использовать double литерал 2.0, то деление перестаёт быть целочисленным, и как следствие, коллизия охватит именно тот радиус, который мы желаем.
А ещё автору статьи ради подтверждения правильности срабатывания пришлось "совершить запрос в космос":
Очень важно то, что в оригинальной версии модификации для Minecraft 1.12.2 эту ошибку исправили. К сожалению, я не смогу предоставить прямые доказательства, поэтому давайте представим, что я совершил запрос в космос, который мне вернул фрагмент кода:
....
this.npc.world.getEntitiesWithinAABB(
EntityLivingBase.class,
npc.getEntityBoundingBox().expand(range, range / 2.0, range)
);
....То есть баг был исправлен в оригинальной модификации, и тут надо сделать то же самое.
Мы переходим к нашему золотому призёру. И им будет... также фрагмент из CustomNPC+. Связано ли обилие Minecraft ошибок с тем, что половина нашей команды занималась модами и плагинами для него? Кто знает.
Переходим к коду:
....
int startIndex = -1;
boolean number = false;
try {
startIndex = Integer.parseInt(bonusID);
number = true;
} catch (Exception var34) {
number = false;
}
for (startIndex = 0; startIndex < bonuses.length; ++startIndex) {
....
if (number && startIndex == startIndex || // <=
!number && bonusValues[startIndex][0].equals(bonusID)
) {
noNBTText = bonusValues[startIndex][0] + ";" + bonusValueString;
bonuses[startIndex] = "";
bonuses[startIndex] = noNBTText;
....
break;
}
}
....Срабатывания анализатора PVS-Studio:
V6001 There are identical sub-expressions 'startIndex' to the left and to the right of the '==' operator. ScriptDBCPlayer.java 289
V6007 Expression '!number' is always true. ScriptDBCPlayer.java 289
V6033 An item with the same key 'startIndex' has already been changed. ScriptDBCPlayer.java 293
Действительно ли срабатывания на этот код указывают на ошибку? И если да, то к чему она приводит? Здесь нас ожидает целое детективное расследование с интересным финалом.
Для начала давайте рассмотрим первое срабатывание. Сравнение startIndex с самим собой действительно кажется странным. Автор статьи, надев свою детективную шляпу и длинный плащ, пошёл искать улики в окрестностях этого класса. Улики лежали совсем неподалёку:
Файл: ScriptDBCPlayer.java(224):
....
int num = -1;
boolean number;
try {
num = Integer.parseInt(bonusID);
number = true;
} catch (Exception var33) {
number = false;
}
for (int i = 0; i < bonuses.length; ++i) {
....
if (number && i == num ||
!number && bonusValues[i][0].equals(bonusID)
) {
bonuses[i] = "";
break;
}
}
....Очень похожий код, но не зря автор статьи имеет прозвище "зоркий глаз". Отличие заключается в том, что во втором случае переменные в try блоке и в цикле for разные: num и i. В первом же фрагменте мы сначала инициализируем startIndex в try блоке, а потом перетираем её в цикле for.
Поэтому во втором фрагменте в идентичной проверке сравниваются две разные переменные: num и i. А в первом имеем то, на что указал анализатор — startIndex == startIndex.
Хорошо, мы поняли, как первый фрагмент должен был выглядеть.
Но на что ошибка влияет?
Тут детектив обратил внимание на название самого класса: префикс Script означает, что класс применяется в механизме игровых сценариев. Сам метод, в котором обнаружена ошибка, ответственен за выставление бонусов игровым атрибутам, таким как сила, ловкость и т.д. По логике этого метода ожидалось, что во внутриигровом скрипте мы выставим определённый индекс бонуса атрибута. Как оно должно было быть?
Сначала мы должны были в блоке try спарсить индекс бонуса:
try {
num = Integer.parseInt(bonusID);
number = true;
} ....А потом в цикле выставить соответствующему бонусу переданное значение:
for (int i = 0; i < bonuses.length; ++i) {
....
if (number && i == num || ....) {
noNBTText = bonusValues[i][0] + ";" + bonusValueString;
bonuses[i] = "";
bonuses[i] = noNBTText;
....
break;
}
}Но во фрагменте с ошибкой полученный из скрипта индекс атрибута перетирается нулём, а потом сравнивается с самим собой:
try {
startIndex = Integer.parseInt(bonusID);
number = true;
} catch (Exception var34) {
number = false;
}
for (startIndex = 0; startIndex < bonuses.length; ++startIndex) {
....
if (number && startIndex == startIndex || // <=
!number && bonusValues[startIndex][0].equals(bonusID)
) {
noNBTText = bonusValues[startIndex][0] + ";" + bonusValueString;
....
}
}То есть переданное в скрипте значение бонуса всегда выставляется первому бонусу. А всё из-за того, что startIndex в for условии перетирается и становится равным нулю, а потом сравнивается сам с собой. А теперь, раз проблема понятна, предлагаю посмотреть на последствия ошибки.
В первую очередь нам нужно написать внутриигровой скрипт, который создаст бонусы атрибуту "Сила".
Сначала создадим ему два бонуса со значениями 1, а потом выставим каждому из них значения 5 и 15 соответственно:
Вот так выглядят внутриигровые скрипты. Мы создали бонусы для "Силы" с индексами 0 и 1, а потом выставили им желаемые значения.
Теперь, по идее, если вывести эти два бонуса у атрибута "Сила", мы увидим 5 и 15? Если бы ошибки в коде не было, так бы и произошло. Однако, как мы поняли из блока выше, ошибка приводит к перетиранию значения у первого бонуса.
Чтобы проверить, давайте напишем следующий внутриигровой скрипт для вывода значений бонусов:
Результат:
Действительно, второй бонус у "Силы" также имеет начальное значение 1. В то время как первый бонус — 15. Получается, что сначала мы выставили первому бонусу значение 5, а потом перетёрли его значением 15. Наша гипотеза выше оказалась правдивой. Дело раскрыто.
За необходимость повозиться и возможность таким вот образом воспроизвести эту ошибку мы награждаем её почётным первым местом!
Вот мы и рассмотрели 10, на наш взгляд, самых интересных ошибок. Надеюсь, вам понравилось! Если есть мысли по поводу этого топа, и вы хотите ими поделиться, будем рады видеть вас в комментариях.
Также, если вам интересна проверка отдельных проектов, то можете найти их в нашем блоге или на этой странице.
Если хотите попробовать сам анализатор PVS-Studio, сделать это можно по этой ссылке. А если вы студент, преподаватель или обладатель Open Source проекта на C, C++, C# или Java, вы можете использовать PVS-Studio бесплатно. Все подробности тут.
Мне остаётся лишь попрощаться с вами и пожелать вам счастливого Нового Года! До скорых встреч!
0
0
0
49
0
42
0
156
0
0