Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Топ 10 ошибок в проектах Java за 2020 г…

Топ 10 ошибок в проектах Java за 2020 год

28 Дек 2020

Новый год неумолимо приближается - а, значит, настало время подводить итоги. Продолжая традицию, мы прошлись по нашим статьям о проверках Java-проектов из мира open-source за этот год и составили рейтинг 10 самых интересных ошибок.

0788_Top_10_Java_bugs_2020_ru/image1.png

За уходящий год мы (Java-команда PVS-Studio) разобрали в наших статьях ошибки из пяти open-source проектов и совсем немного рассказали про нашу внутреннюю кухню:

Предлагаем читателю сначала ознакомиться с этими статьями и составить свой личный рейтинг, чтобы потом сравнить его с нашим и сказать, что мы неправы :).

Десятое место: "Обманчивое равенство"

Источник: Big/Bug Data: анализируем исходный код Apache Flink

V6001 There are identical sub-expressions 'processedData' to the left and to the right of the '==' operator. CheckpointStatistics.java(229)

@Override
public boolean equals(Object o) 
{
  ....
  CheckpointStatistics that = (CheckpointStatistics) o;
  return id == that.id &&
    savepoint == that.savepoint &&
    triggerTimestamp == that.triggerTimestamp &&
    latestAckTimestamp == that.latestAckTimestamp &&
    stateSize == that.stateSize &&
    duration == that.duration &&
    alignmentBuffered == that.alignmentBuffered &&
    processedData == processedData &&                // <=
    persistedData == that.persistedData &&
    numSubtasks == that.numSubtasks &&
    numAckSubtasks == that.numAckSubtasks &&
    status == that.status &&
    Objects.equals(checkpointType, that.checkpointType) &&
    Objects.equals(
      checkpointStatisticsPerTask, 
      that.checkpointStatisticsPerTask);
}

Простая и очень обидная ошибка из-за невнимательности: поле processedData сравнивается с самим собой. Из-за этой ошибки сравнение объектов типа CheckpointStatistics иногда будет выдавать ложноположительный результат. Но основная опасность этой опечатки состоит в том, что equals крайне активно используется в коллекциях, и некорректная реализация этого метода может привести к очень странному поведению, на отладку которого уйдёт огромное количество времени.

Хочу заметить, что ошибаться в функциях сравнения для разработчиков привычное дело. Мой коллега даже написал большую статью "Зло живет в функциях сравнения" с множеством примеров и объяснений.

Девятое место: "Недостижимый код"

Источник: Единороги на страже вашей безопасности: исследуем код Bouncy Castle.

V6019 Unreachable code detected. It is possible that an error is present. XMSSTest.java(170)

public void testSignSHA256CompleteEvenHeight2() {
    ....
    int height = 10;
    ....
    for (int i = 0; i < (1 << height); i++) {
        byte[] signature = xmss.sign(new byte[1024]);
        switch (i) {
            case 0x005b:
                assertEquals(signatures[0], Hex.toHexString(signature));
                break;
            case 0x0822:
                assertEquals(signatures[1], Hex.toHexString(signature));
                break;
            ....
        }
    }
}

Ветвь switch для значения i == 0x0822(2082) оказалась недостижимой. Как же так получилось?

Если обратить внимание на условие цикла 1 << height , где height всегда равен 10, то всё сразу встанет на свои места. Согласно условию цикла, счётчик i в цикле for не может быть больше, чем 1024 (1 << 10). Естественно, выполнение рассматриваемой ветви switch никогда не произойдет.

Восьмое место: "Проаннотированный метод"

Источник: Под капотом PVS-Studio для Java: разработка диагностик.

V6009 Collection is empty. The call of the 'clear' function is senseless. MetricRepositoryRule.java(90)

protected void after()
{
  this.metricsById.clear();
  this.metricsById.clear();
}

Часть наших диагностик сильно полагается на механизм аннотирования методов. Аннотации предоставляют дополнительную информацию анализатору об используемых методах, например:

  • Чистый ли это метод,
  • Какие накладываются ограничения на аргументы,
  • Возвращаемый результат,
  • ... и всякое прочее.

Некоторые аннотации анализатор выводит сам из исходного кода, некоторые мы проставляем вручную (например, для методов стандартной библиотеки). История этой ошибки началась с того, что мы не в полной мере проаннотировали метод Map#clear. После того, как мы это заметили и исправили, на наших тестовых проектах повылезали новые срабатывания, среди которых был и наш интересный случай.

На первый взгляд, повторная очистка словаря – не ошибка. И мы бы даже решили, что это случайно продублированная строка, если бы не обратили внимание на поля класса:

private final Map<String, Metric> metricsByKey = new HashMap<>();
private final Map<Long, Metric> metricsById = new HashMap<>();

У класса есть два поля с похожими именами metricsById и metricsByKey. Это и наталкивает на мысль, что автор кода хотел очистить оба словаря, но ... этого не произошло. Таким образом, два словаря, которые хранят связанные данные, будут рассинхронизированы после вызова after.

Седьмое место: "Ожидание / реальность"

Источник: Проверка WildFly - сервера JavaEE приложений.

V6058 The 'equals' function compares objects of incompatible types: String, ModelNode. JaxrsIntegrationProcessor.java(563)

// Send value to RESTEasy only if it's not null, empty string, or the 
// default value.
private boolean isTransmittable(AttributeDefinition attribute,
                                ModelNode modelNode) {
  if (modelNode == null || ModelType
      .UNDEFINED.equals(modelNode.getType())) {
    return false;
  }
  String value = modelNode.asString();
  if ("".equals(value.trim())) {
    return false;
  }
  return !value.equals(attribute.getDefaultValue());        // <=
}

Обратив внимание на предшествующий методу комментарий, можно ожидать, что метод вернет true, если:

  • modelNode не null,
  • строковое представление modelNode не пустое,
  • modelNode – не значение по умолчанию.

Несмотря на авторский комментарий и корректную, на первый взгляд, логику, поведение метода будет отличаться. Причиной этому служит проверка modelNode на равенство со значением по умолчанию в последней строчке метода.

Строковое представление modelNode сравнивается с объектом типа ModelNode, и, как можно догадаться, такое сравнение всегда будет возвращать отрицательный результат из-за несовместимости типов.

Последствия ошибки: непредвиденное разрешение к отправке значения modelNode, когда оно равно значению по умолчанию (attribute.getDefaultValue()).

Шестое место: "Копипаст-ориентированное программирование"

Источник: Проверка кода XMage и почему недоступны специальные редкие карточки для коллекции Dragon's Maze.

V6072 Two similar code fragments were found. Perhaps, this is a typo and 'playerB' variable should be used instead of 'playerA'. SubTypeChangingEffectsTest.java(162), SubTypeChangingEffectsTest.java(158), SubTypeChangingEffectsTest.java(156), SubTypeChangingEffectsTest.java(160)

@Test
public void testArcaneAdaptationGiveType() {
    addCard(Zone.HAND, playerA, "Arcane Adaptation", 1);
    addCard(Zone.BATTLEFIELD, playerA, "Island", 3);

    addCard(Zone.HAND, playerA, "Silvercoat Lion");
    addCard(Zone.BATTLEFIELD, playerA, "Silvercoat Lion");
    addCard(Zone.GRAVEYARD, playerA, "Silvercoat Lion");   // <=

    addCard(Zone.HAND, playerB, "Silvercoat Lion");
    addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion");
    addCard(Zone.GRAVEYARD, playerA, "Silvercoat Lion");   // <=

    ....

    for (Card card : playerB.getGraveyard().getCards(currentGame)) {
        if (card.isCreature()) {
            Assert.assertEquals(card.getName() + " should not have ORC type",
                    false, card.getSubtype(currentGame).contains(SubType.ORC));
            Assert.assertEquals(card.getName() + " should have CAT type",
                    true, card.getSubtype(currentGame).contains(SubType.CAT));
        }
    }
}

В этом году, как и в прошлом (Топ 10 ошибок за 2019), классная copy-paste ошибка от диагностического правила V6072 заслуживает место в десятке.

Характер ошибки заключается в том, что когда разработчику нужно сделать схожие действия для различных переменных, то он копирует добросовестно написанный ранее код и изменяет название переменной. Но делает это уже не совсем добросовестно и забывает доизменить переменные.

В данном фрагменте кода так и произошло. Автор теста имитировал игру между игроками, раскидывая между ними одинаковые карты по игровым зонам, но из-за copy-paste игроку playerA дважды досталась одна и та же карта. Из-за этого игровая зона Zone.GRAVEYARD игрока playerB осталась без тестирования. Подробное описание ошибки можно почитать в самой статье.

Пятое место: "Ненормальное распределение"

Источник: Big/Bug Data: анализируем исходный код Apache Flink

V6048 This expression can be simplified. Operand 'index' in the operation equals 0. CollectionUtil.java(76)

public static <T> 
Collection<List<T>> partition(Collection<T> elements, int numBuckets) 
{
  Map<Integer, List<T>> buckets = new HashMap<>(numBuckets);
  
  int initialCapacity = elements.size() / numBuckets;

  int index = 0;
  for (T element : elements) 
  {
    int bucket = index % numBuckets;                                 // <=
    buckets.computeIfAbsent(bucket, 
                            key -> new ArrayList<>(initialCapacity))

           .add(element); 
  }

  return buckets.values();
}

Ошибка была обнаружена в утилитном методе partition, который разбивает переданную коллекцию elements на numBuckets коллекций. Суть ошибки в том, что индекс коллекции bucket, в которую хотят поместить каждый рассматриваемый элемент, имеет константное значение (0). Причиной этому служит то, что разработчик забыл инкрементировать переменную index на каждой итерации цикла.

Вследствие чего метод partition будет всегда возвращать коллекцию elements, обернутую в другую коллекцию. А это – вряд ли задуманное поведение.

Четвертое место: "Бомба замедленного действия"

Источник: АНБ, Ghidra и единороги.

V6008 Null dereference of 'selectedNode' in function 'setViewPanel'. OptionsPanel.java(266)


private void processSelection(OptionsTreeNode selectedNode) {
  if (selectedNode == null) {
    setViewPanel(defaultPanel, selectedNode); // <=
    return;
  }
  ....
}
private void setViewPanel(JComponent component, OptionsTreeNode selectedNode) {
  ....
  setHelpLocation(component, selectedNode);
  ....
}
private void setHelpLocation(JComponent component, OptionsTreeNode node) {
  Options options = node.getOptions();
  ....
}

В приведенном фрагменте кода явно напортачили. Если вы проследите за selectedNode из processSelection(), когда selectedNode == null, то сразу же обнаружите, что при таком исходе нас ждет неминуемый NullPointerException. О чем и предупреждает нас анализатор.

Но, изучив немного код, автор статьи пришел к выводу, что выполнение программы никогда не встретится с NullPointerException, так как processSelection() вызывается всего в двух местах, перед вызовом которых selectedNode явно проверяется на null.

Несмотря на это, такой код – бомба замедленного действия, поскольку другой разработчик может увидеть, что метод явно обрабатывает случай selectedNode == null, и решить, что это валидное значение, что потом выльется в падение приложения.

Третье место: "Всегда false"

Источник: Проверка кода XMage и почему недоступны специальные редкие карточки для коллекции Dragon's Maze.

V6007 Expression 'filter.getMessage().toLowerCase(Locale.ENGLISH).startsWith("Each ")' is always false. SetPowerToughnessAllEffect.java(107)

@Override
public String getText(Mode mode) {
  StringBuilder sb = new StringBuilder();
  ....
  if (filter.getMessage().toLowerCase(Locale.ENGLISH).startsWith("Each ")) {
    sb.append(" has base power and toughness ");
  } else {
    sb.append(" have base power and toughness ");
  }
  ....
  return sb.toString();
}

Кто же сравнивает строку, приведенную в нижний регистр, со строкой, которая начинается с заглавной буквы? Отсюда и всегда ложный результат проверки сообщения.

Результат дефекта не критический, но тоже неприятный: где-то будет фигурировать неграмотно составленный текст.

Второе место: "2-в-1"

Источник: АНБ, Ghidra и единороги.

V6007 Expression 'index >= 0' is always true. ExternalNamesTableModel.java(105)

V6019 Unreachable code detected. It is possible that an error is present. ExternalNamesTableModel.java(109)

public void setValueAt(Object aValue, int row, int column) {
  ....
  int index = indexOf(newName);
  if (index >= 0) {                  // <=
    Window window = tool.getActiveWindow();
    Msg.showInfo(getClass(), window, "Duplicate Name",
                 "Name already exists: " + newName);
    return;
  }

  ExternalPath path = paths.get(row); // <=
  ....
}

private int indexOf(String name) {
  for (int i = 0; i < paths.size(); i++) {
    ExternalPath path = paths.get(i);
    if (path.getName().equals(name)) {
      return i;
    }
  }
  return 0;
}

Метод indexOf всегда возвращает неотрицательное число. А всё из-за того, что автор метода в случае отсутствия искомого newName по ошибке возвращает 0, а не -1. Такая ошибка приводит к тому, что поток выполнения программы всегда будет заходить в then-ветку условного оператора if (index >= 0), в котором будет выдавать сообщение о существующем newName и успешно выходить из метода, даже тогда, когда в реальности newName не был найден.

Но и это ещё не всё. Так как then-ветка условного оператора прекращает выполнение метода, то до кода после условного оператора дело так и не дойдет.

Об этом и предупреждает нас анализатор.

Первое место: "А то ли мы проверили?"

Источник: Под капотом PVS-Studio для Java: разработка диагностик.

V6080 Consider checking for misprints. It's possible that an assigned variable should be checked in the next condition. Menu.java(40)

public class Menu
{
  private Map<String, List<String>> menus = new HashMap<String, List<String>>();

  public void putMenuItem(String menu, String item)
  {
    List<String> items = menus.get(menu);
    if (item == null)                      // <=
    {
      items = new ArrayList<String>();
      menus.put(menu, items);
    }
    items.add(item);
  }
  ....
}

По задумке автора предполагалось создать коллекцию по ключу menu, если таковой ещё не было. Но проверка не той переменной разрушила всю задумку, прорубив лазеечку для NullPointerException. Метод выбросит исключение, когда в словаре ключ menu будет отсутствовать, и значение item, которое хотели добавить, не будет null.

Заключение

Проверки open-source проектов с помощью PVS-Studio из года в год доказывают, что такой рубеж защиты, как статический анализ кода, должен обязательно присутствовать в разработке. Каким бы вы мастером своего дела ни были, ошибки обязательно найдут лазеечку в ваш проект, и причин этому множество: устали, завал на работе или вовсе отвлеклись на котиков. А если вы работаете в команде, то количество возможностей попасть ошибкам в код вырастает пропорционально количеству коллег.

Если вам понравился наш обзор, то не ждите следующего конца года. Статьи о проверках начнутся сразу же с первого месяца 2021, а если вам не терпится – смелее скачивайте анализатор и самостоятельно проверяйте open-source проекты.

Популярные статьи по теме
Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Дата: 22 Окт 2018

Автор: Андрей Карпов

PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь ок…
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

Автор: Андрей Карпов

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

Автор: Андрей Карпов

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
Зло живёт в функциях сравнения

Дата: 19 Май 2017

Автор: Андрей Карпов

Возможно, читатели помнят мою статью под названием "Эффект последней строки". В ней идёт речь о замеченной мной закономерности: ошибка чаще всего допускается в последней строке однотипных блоков текс…
Эффект последней строки

Дата: 31 Май 2014

Автор: Андрей Карпов

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

Автор: Андрей Карпов

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

Автор: Андрей Карпов

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

Автор: Андрей Карпов

Краткое описание технологий, используемых в инструменте PVS-Studio, которые позволяют эффективно обнаруживать большое количество паттернов ошибок и потенциальных уязвимостей. Статья описывает реализа…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

Автор: Андрей Карпов

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Любите статический анализ кода!

Дата: 16 Окт 2017

Автор: Андрей Карпов

Я в шоке от возможностей статического анализа кода, хотя сам участвую в разработке инструмента PVS-Studio. На днях я был искренне удивлён тому, что анализатор оказался умнее и внимательнее меня.

Комментарии (0)

Следующие комментарии

На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Этот сайт использует куки и другие технологии, чтобы предоставить вам более персонализированный опыт. Продолжая просмотр страниц нашего веб-сайта, вы принимаете условия использования этих файлов. Если вы не хотите, чтобы ваши данные обрабатывались, пожалуйста, покиньте данный сайт. Подробнее →
Принять