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

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

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

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

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

Ваше сообщение отправлено.

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


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

>
>
>
Huawei Cloud: в PVS-Studio сегодня обла…

Huawei Cloud: в PVS-Studio сегодня облачно

26 Ноя 2019

В нынешний век каждый уже слышал об облачных сервисах. Многие компании освоили этот сегмент рынка и создали свои облачные сервисы самых различных направлений. Наша команда также в последнее время интересуется этими сервисами с точки зрения интеграции с ними анализатора кода PVS-Studio. Думаем, наши постоянные читатели уже догадываются, проект какого типа мы проверим в этот раз. Выбор пал на код облачных сервисов компании Huawei.

0688_HuaweiCloud_ru/image1.png

Введение

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

И вот когда я подбирал очередной интересный проект для грядущей статьи, мне на почту пришло предложение о работе от компании Huawei. При сборе информации о данной компании выяснилось, что у них есть свои облачные сервисы, но главное то, что исходный код этих сервисов лежит в свободном доступе на GitHub. Именно это стало главной причиной выбора этой компании для данной статьи. Как говорил один китайский мудрец: "Случайности не случайны".

Теперь немного о нашем анализаторе. PVS-Studio - это статический анализатор кода, который выявляет ошибки и уязвимости в исходном коде программ написанных на C, C++, C# и Java. Анализатор работает на платформах Windows, Linux и macOS. Помимо плагинов к таким классическим средам разработки, как Visual Studio или IntelliJ IDEA, анализатор имеет возможность интегрироваться с SonarQube и Jenkins:

Анализ проекта

В процессе поиска информации для статьи я выяснил, что у Huawei имеется developer center, где можно получить информацию, руководства и исходники их облачных сервисов. Для создания этих сервисов использовались разнообразные языки программирования, но самыми популярными оказались такие языки, как Go, Java и Python.

Так как я специализируюсь на Java, то и проекты были выбраны соответствующие. Исходники проектов, анализируемых в статье, можно получить в GitHub репозитории huaweicloud.

Для анализа проектов мне потребовалось совершить лишь несколько действий:

  • Получить проекты из репозитория;
  • Воспользоваться инструкцией по запуску Java-анализатора и запустить анализ на каждом из проектов.

Проанализировав проекты, получилось выделить лишь три, которым хотелось бы уделить внимание. Это является следствием того, что размер остальных Java проектов оказался слишком мал.

Результаты анализа проектов (количество предупреждений и количество файлов):

Предупреждений немного, так что в целом можно сказать о хорошем качестве кода. Тем более, что не все срабатывания являются действительными ошибками. Связанно это с тем, что анализатору иногда не хватает информации, чтобы отличить корректный код от ошибочного. Поэтому диагностики анализатора совершенствуются каждый день при помощи информации, полученной от пользователей. См. также статью "Как и почему статические анализаторы борются с ложными срабатываниями".

В процессе анализа проектов я отобрал самые интересные предупреждения, о которых и расскажу в данной статье.

Порядок инициализации полей

V6050 Class initialization cycle is present. Initialization of 'INSTANCE' appears before the initialization of 'LOG'. UntrustedSSL.java(32), UntrustedSSL.java(59), UntrustedSSL.java(33)

public class UntrustedSSL {
  
  private static final UntrustedSSL INSTANCE = new UntrustedSSL();
  private static final Logger LOG = LoggerFactory.getLogger(UntrustedSSL.class);
  .... 
  private UntrustedSSL() 
  {
    try
    {
      ....
    }
    catch (Throwable t) {
      LOG.error(t.getMessage(), t);           // <=
    }
  }
}

При возникновении любого исключения в конструкторе класса UntrustedSSL, информация об этом исключении логируется в блоке catch при помощи логгера LOG. Однако из-за порядка инициализации статических полей, LOG в момент инициализации поля INSTANCE еще не инициализирован. Поэтому при логировании информации об исключении в конструкторе произойдет исключение NullPointerException. Это исключение является причиной еще одного исключения ExceptionInInitializerError, которое выбрасывается, если произошло исключение при инициализации статического поля. Все что необходимо для решения описанной проблемы - поместить инициализацию LOG перед инициализацией INSTANCE.

Незаметная опечатка

V6005 The variable 'this.metricSchema' is assigned to itself. OpenTSDBSchema.java(72)

public class OpenTSDBSchema
{
  @JsonProperty("metric")
  private List<SchemaField> metricSchema;
  ....
  public void setMetricsSchema(List<SchemaField> metricsSchema)
  {
    this.metricSchema = metricSchema;           // <=
  }
   
  public void setMetricSchema(List<SchemaField> metricSchema)
  {
    this.metricSchema = metricSchema;
  }
  ....
}

Оба метода устанавливают поле metricSchema, но названия методов различаются на один символ 's'. Программист именовал аргументы этих методов в соответствии с названием метода. В результате в строке, на которую указывает анализатор, поле metricSchema присваивается самому себе, а аргумент метода metricsSchema не используется.

V6005 The variable 'suspend' is assigned to itself. SuspendTransferTaskRequest.java(77)

public class SuspendTransferTaskRequest 
{
  ....
  private boolean suspend;
  ....
  public void setSuspend(boolean suspend)
  {
    suspend = suspend;                        
  }
  .... 
}

Здесь представлена банальная ошибка, связанная с невнимательностью, из-за которой происходит присваивание аргумента suspend самому себе. В результате полю suspend не будет присвоено значение пришедшего аргумента, как подразумевалось. Правильно:

public void setSuspend(boolean suspend)
{
  this.suspend = suspend;                        
}

Предопределенность условий

Как это часто бывает, правило V6007 вырывается вперед по количеству предупреждений.

V6007 Expression 'firewallPolicyId == null' is always false. FirewallPolicyServiceImpl.java(125)

public FirewallPolicy
removeFirewallRuleFromPolicy(String firewallPolicyId,
                             String firewallRuleId) 
{
  checkNotNull(firewallPolicyId);
  checkNotNull(firewallRuleId);
  checkState(!(firewallPolicyId == null && firewallRuleId == null),
  "Either a Firewall Policy or Firewall Rule identifier must be set"); 
  .... 
}

В этом методе аргументы проверяются на равенство null методом checkNotNull:

@CanIgnoreReturnValue
public static <T> T checkNotNull(T reference) 
{
  if (reference == null) {
    throw new NullPointerException();
  } else {
    return reference;
  }
}

После проверки аргумента методом checkNotNull, на 100% можно быть уверенным в том, что аргумент, переданный в этот метод, не равен null. Так как оба аргумента метода removeFirewallRuleFromPolicy проверяются методом checkNotNull, то не имеет смысла в дальнейшем проверять аргументы на равенство null еще раз. Однако в метод checkState в качестве первого аргумента передается выражение, где аргументы firewallPolicyId и firewallRuleId повторно проверяются на равенство null.

Аналогичное предупреждение выдается и для firewallRuleId:

  • V6007 Expression 'firewallRuleId == null' is always false. FirewallPolicyServiceImpl.java(125)

V6007 Expression 'filteringParams != null' is always true. NetworkPolicyServiceImpl.java(60)

private Invocation<NetworkServicePolicies> buildInvocation(Map<String,
String> filteringParams) 
{
  .... 
  if (filteringParams == null) {
    return servicePoliciesInvocation;
  }
  if (filteringParams != null) {       // <=
    ....
  }
  return servicePoliciesInvocation;
}

В данном методе, если аргумент filteringParams равняется null, то происходит выход из метода с возвращением значения. Поэтому проверка, на которую указывает анализатор, всегда будет истинной, а значит эта проверка не имеет смысла.

Подобное встречается еще в 13 классах:

  • V6007 Expression 'filteringParams != null' is always true. PolicyRuleServiceImpl.java(58)
  • V6007 Expression 'filteringParams != null' is always true. GroupServiceImpl.java(58)
  • V6007 Expression 'filteringParams != null' is always true. ExternalSegmentServiceImpl.java(57)
  • V6007 Expression 'filteringParams != null' is always true. L3policyServiceImpl.java(57)
  • V6007 Expression 'filteringParams != null' is always true. PolicyRuleSetServiceImpl.java(58)
  • и так далее . . .

Нулевая ссылка

V6008 Potential null dereference of 'm.blockDeviceMapping'. NovaServerCreate.java(390)

@Override
public ServerCreateBuilder blockDevice(BlockDeviceMappingCreate blockDevice) {
  if (blockDevice != null && m.blockDeviceMapping == null) {
    m.blockDeviceMapping = Lists.newArrayList();
  }
  m.blockDeviceMapping.add(blockDevice);       // <=
  return this;
}

В данном методе инициализация ссылочного поля m.blockDeviceMapping не произойдет, если аргумент blockDevice будет равен null. Это поле инициализируется только в данном методе, поэтому при вызове метода add у поля m.blockDeviceMapping произойдет исключение NullPointerException.

V6008 Potential null dereference of 'FileId.get(path)' in function '<init>'. TrackedFile.java(140), TrackedFile.java(115)

public TrackedFile(FileFlow<?> flow, Path path) throws IOException 
{
  this(flow, path, FileId.get(path), ....);
}

В качестве третьего аргумента конструктору класса TrackedFile передается результат статического метода FileId.get(path). Но этот метод может вернуть null:

public static FileId get(Path file) throws IOException
{
  if (!Files.exists(file))
  {
    return null;
  }
  ....
}

В конструкторе, вызываемом через this, аргумент id не изменяется до его первого использования:

public TrackedFile(...., ...., FileId id, ....) throws IOException
{
  ....
  FileId newId = FileId.get(path);
  if (!id.equals(newId))
  {
    ....
  }
}

Следовательно, если в метод в качестве третьего аргумента будет передан null, то произойдет исключение.

Подобная ситуация происходит еще в одном случае:

  • V6008 Potential null dereference of 'buffer'. PublishingQueue.java(518)

V6008 Potential null dereference of 'dataTmpFile'. CacheManager.java(91)


@Override
public void putToCache(PutRecordsRequest putRecordsRequest)
{
  .... 
  if (dataTmpFile == null || !dataTmpFile.exists())
  {
    try
    {
      dataTmpFile.createNewFile();  // <=
    }
    catch (IOException e)
    {
      LOGGER.error("Failed to create cache tmp file, return.", e);
      return ;
    }
  }
  ....
}

И снова NPE. Ряд проверок в условном операторе вполне допускает нулевой объект dataTmpFile для дальнейшего разыменования. Я думаю, здесь две опечатки, и проверка, на самом деле, должна быть такой:

if (dataTmpFile != null && !dataTmpFile.exists())

Подстроки и отрицательные числа

V6009 The 'substring' function could receive the '-1' value while non-negative value is expected. Inspect argument: 2. RemoveVersionProjectIdFromURL.java(37)

@Override
public String apply(String url) {
  String urlRmovePojectId = url.substring(0, url.lastIndexOf("/"));
  return urlRmovePojectId.substring(0, urlRmovePojectId.lastIndexOf("/"));
}

Подразумевается, что в данный метод передается URL в виде строки, которая никак не валидируется. После эту строку обрезают несколько раз при помощи метода lastIndexOf. Если метод lastIndexOf не найдет совпадения в строке, то вернет -1. Это приведет к исключению StringIndexOutOfBoundsException, так как аргументы метода substring должны быть неотрицательными числами. Для корректной работы метода необходимо добавить валидацию входного аргумента или проверить, что результаты методов lastIndexOf не являются отрицательными числами.

Вот еще места, где встречается подобное:

  • V6009 The 'substring' function could receive the '-1' value while non-negative value is expected. Inspect argument: 2. RemoveProjectIdFromURL.java(37)
  • V6009 The 'substring' function could receive the '-1' value while non-negative value is expected. Inspect argument: 2. RemoveVersionProjectIdFromURL.java(38)

Забытый результат

V6010 The return value of function 'concat' is required to be utilized. AKSK.java(278)

public static String buildCanonicalHost(URL url) 
{
  String host = url.getHost();
  int port = url.getPort();
  if (port > -1) {
    host.concat(":" + Integer.toString(port));
  }
  return host;
}

При написании этого кода не было учтено, что вызов метода concat не изменит строку host ввиду неизменяемости объектов типа String. Для корректной работы метода необходимо присвоить результат метода concat переменной host в блоке if. Правильно:

if (port > -1) {
  host = host.concat(":" + Integer.toString(port));
}

Неиспользуемые переменные

V6021 Variable 'url' is not used. TriggerV2Service.java(95)

public ActionResponse deleteAllTriggersForFunction(String functionUrn) 
{
  checkArgument(!Strings.isNullOrEmpty(functionUrn), ....);
  String url = ClientConstants.FGS_TRIGGERS_V2 +
               ClientConstants.URI_SEP + 
               functionUrn;
  return deleteWithResponse(uri(triggersUrlFmt, functionUrn)).execute();
}

В данном методе переменная url не используется после инициализации. Скорее всего, в метод uri вторым аргументом должна была передаваться переменная url вместо functionUrn, так как переменная functionUrn участвует в инициализации переменной url.

Аргумент, не используемый в конструкторе

V6022 Parameter 'returnType' is not used inside constructor body. HttpRequest.java(68)

public class HttpReQuest<R> 
{
  ....
  Class<R> returnType;
  ....
  public HttpRequest(...., Class<R> returnType) // <=
  {      
    this.endpoint = endpoint;
    this.path = path;
    this.method = method;
    this.entity = entity;
  }
  ....
  public Class<R> getReturnType() 
  {
    return returnType;
  }
  ....
}

В этом конструкторе забыли использовать аргумент returnType и присвоить его значение полю returnType. Поэтому при вызове метода getReturnType у объекта, созданного данным конструктором, будет возвращено значение по умолчанию – null, хотя, скорее всего, подразумевалось получить объект, ранее переданный в конструктор.

Одинаковая функциональность

V6032 It is odd that the body of method 'enable' is fully equivalent to the body of another method 'disable'. ServiceAction.java(32), ServiceAction.java(36)

public class ServiceAction implements ModelEntity 
{    
  private String binary;
  private String host;

  private ServiceAction(String binary, String host) {
    this.binary = binary;
    this.host = host;
  }

  public static ServiceAction enable(String binary, String host) { // <=
    return new ServiceAction(binary, host);
  }

  public static ServiceAction disable(String binary, String host) { // <=
    return new ServiceAction(binary, host);
  }
  ....
}

Наличие двух одинаковых методов не является ошибкой, но то, что два метода выполняют одно и то же действие – это, как минимум, странно. Взглянув на названия вышеуказанных методов можно предположить, что они выполняют противоположные действия. На самом деле, оба метода выполняют одно и то же – создают и возвращают объект ServiceAction. Скорее всего, метод disable создали копированием кода метода enable, но при этом забыли изменить тело метода.

Забыли проверить главное

V6060 The 'params' reference was utilized before it was verified against null. DomainService.java(49), DomainService.java(46)

public Domains list(Map<String, String> params)
{
  Preconditions.checkNotNull(params.get("page_size"), ....);
  Preconditions.checkNotNull(params.get("page_number"), ....);
  Invocation<Domains> domainInvocation = get(Domains.class, uri("/domains"));
  if (params != null) {                                      // <=
    ....
  }
  return domainInvocation.execute(this.buildExecutionOptions(Domains.class));
}

В этом методе решили проверить содержимое структуры типа Map на неравенство null. Для этого у аргумента params два раза вызывают метод get, результат которого передается в метод checkNotNull. Все кажется логичным, но как бы не так! В if происходит проверка аргумента params на неравенство null. После чего можно предположить, что входной аргумент может быть равен null, но до этой проверки у params уже два раза был вызван метод get. Если в качестве аргумента в данный метод будет передан null, то при первом же вызове метода get будет выброшено исключение.

Аналогичная ситуация встречается еще в трех местах:

  • V6060 The 'params' reference was utilized before it was verified against null. DomainService.java(389), DomainService.java(387)
  • V6060 The 'params' reference was utilized before it was verified against null. DomainService.java(372), DomainService.java(369)
  • V6060 The 'params' reference was utilized before it was verified against null. DomainService.java(353), DomainService.java(350)

Заключение

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

PVS-Studio обязательно сообщит компании Huawei о результатах проверки их облачных сервисов, чтобы разработчики этой компании могли изучить их более подробно.

Разовое использование статического анализа кода, продемонстрированное в статье, не может показать все его преимущества. Более подробную информацию о том, как правильно использовать статический анализ, можно посмотреть здесь и здесь. Скачать анализатор PVS-Studio можно тут.

Популярные статьи по теме
PVS-Studio ROI

Дата: 30 Янв 2019

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

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

Дата: 22 Окт 2018

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

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

Дата: 17 Янв 2019

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

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
Зло живёт в функциях сравнения

Дата: 19 Май 2017

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

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

Дата: 14 Апр 2016

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

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

Дата: 27 Июн 2017

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

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

Дата: 22 Дек 2018

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

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

Дата: 21 Ноя 2018

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

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

Дата: 16 Окт 2017

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

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

Дата: 31 Май 2014

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

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

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

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