metrica
Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо
to the top
close form

Заполните форму в два простых шага ниже:

Ваши контактные данные:

Шаг 1
Поздравляем! У вас есть промокод!

Тип желаемой лицензии:

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

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

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

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

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

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


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

Вебинар: Трудности при интеграции SAST, как с ними справляться - 04.04

>
>
>
Нужно ли проверять библиотеки перед их …

Нужно ли проверять библиотеки перед их использованием? Разберём на примере MudBlazor

10 Фев 2023

В нашей компании возникла потребность использования библиотеки для Blazor компонентов. Мы остановились на MudBlazor и перед внедрением проверили качество её кода. В результате нашли ряд странностей и даже воспроизводящееся падение, о чём и расскажем в статье.

1032_MudBlazor_ru/image1.png

MudBlazor — одна из самых популярных библиотек для приложений под Blazor. Во многом наш выбор пал на неё из-за количества компонентов и красоты их реализации. Её исходный код написан на C#, поэтому перед непосредственным использованием мы решили проанализировать его. Для анализа взяли исходники версии v6.1.7. Дальше речь пойдёт о найденных ошибках.

Сомнительный вызов

Issue 1

public EndSlopeSpline(....):base(....)
{
  m = new Matrix(n);
  gauss = new MatrixSolver(n, m);
  a = new double[n];
  b = new double[n];
  c = new double[n];
  d = new double[n];
  h = new double[n];

  CalcParameters(firstSlopeDegrees, lastSlopeDegrees);
  Integrate();                                            // <=
  Interpolate();
}

Предупреждение PVS-Studio: V3010 The return value of function 'Integrate' is required to be utilized. EndSlopeSpline.cs 26

Чтобы разобраться в сути данного срабатывания, необходимо обратиться к реализации метода Integrate:

public double Integrate()
{
  double integral = 0;
  for (int i = 0; i < h.Length; i++)
  {
    double termA = a[i] * h[i];
    double termB = b[i] * Math.Pow(h[i], 2) / 2.0;
    double termC = c[i] * Math.Pow(h[i], 3) / 3.0;
    double termD = d[i] * Math.Pow(h[i], 4) / 4.0;
    integral += termA + termB + termC + termD;
  }
  return integral;
}

В методе не изменяется чьё-либо состояние (например, поля или свойства). Исходя из этого, можно сделать вывод, что единственное назначение этого метода — расчёт с последующим возвращением значения переменной integral. Это делает бессмысленным вызов Integrate без использования возвращаемого значения.

Стоит отметить, что методы CalcParameters и Interpolate (вызванные рядом с Integrate) как раз изменяют состояния ряда свойств и полей.

Issue 2

internal void DateValueChanged(DateTime? value)
{
  _valueDate = value;

  if (value != null)
  {
    var date = value.Value.Date;

    // get the time component and add it to the date.
    if (_valueTime != null)
    {
      date.Add(_valueTime.Value);                    // <=
    }

    _filterDefinition.Value = date;
  }
  else
    _filterDefinition.Value = value;

  _dataGrid.GroupItems();
}

Предупреждение PVS-Studio: V3010 The return value of function 'Add' is required to be utilized. Filter.cs 140

По своей сути срабатывание похоже на предыдущее. Здесь не было использовано возвращаемое значение метода Add, вызванного у переменной типа DateTime. Скорее всего, разработчик хотел добавить значение _valueTime.Value к date. Однако он не учёл, что метод Add для объекта типа DateTime возвращает результат добавления, а не изменяет исходный объект. В итоге имеем бесполезный вызов, который по задумке наверняка должен был на что-то влиять.

Неожиданный NullReferenceException

Issue 3

public static bool operator ==(ResizeOptions l, ResizeOptions r) 
                                                       => l.Equals(r);

public static bool operator !=(ResizeOptions l, ResizeOptions r) 
                                                       => !l.Equals(r);

Предупреждения PVS-Studio:

  • V3115 Passing 'null' to '==' operator should not result in 'NullReferenceException'. ResizeOptions.cs 34
  • V3115 Passing 'null' to '!=' operator should not result in 'NullReferenceException'. ResizeOptions.cs 35

При перегрузке операторов равенства и неравенства стоит учитывать возможность того, что один из операндов может иметь значение null. Подобные рекомендации даны в документации Microsoft.

Однако в реализации перегрузки операторов '==' и '!=' левый операнд не проверяется на null. Если значение параметра lnull, то при вызове метода Equals будет выброшено исключение типа NullReferenceException. Аргумент метода Equals, кстати, так же разыменовывается без проверки:

public bool Equals(ResizeOptions other)
{
  if (ReportRate != other.ReportRate || ....)     // <=
  {
    return false;
  }
  ....
}

Такое поведение может стать весьма неприятным сюрпризом для пользователя, если класс публичный. Именно таким и является ResizeOptions.

Чтобы убедиться в наличии проблемы, попробуем сравнить объект типа ResizeOptions с null.

1032_MudBlazor_ru/image2.png

Вполне ожидаемо получаем исключение типа NullReferenceException. Если правый операнд – null, поведение будет аналогичным.

1032_MudBlazor_ru/image3.png

Необоснованное присваивание

Issue 4

protected override void OnParametersSet()
{
  if (....)
  {
    ....
    foreach (....)
    {
      if (firstTime)
      {
        ....
        gridValueY = verticalStartSpace;                   // <=
      }
      else
      {
        ....
        gridValueY = verticalStartSpace;                   // <=
      }
      gridValueY = yValue;                                 // <=
      ....
    }
  }
  else
  {
    ....
  }
  ....
}

Предупреждение PVS-Studio: V3008 The 'gridValueY' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 191, 189. Line.razor.cs 191

Рассмотрим вложенный if. В обеих ветвях этого оператора переменной gridValueY присваивается некоторое значение. Стоит отметить, что оно одинаково как в первом, так и во втором случае. Получается, что такое присваивание стоило вообще вынести за if. Однако основная проблема заключается не в этом: значение gridValueY изменяется сразу после if, хотя оно не использовалось.

Блок else оператора if верхнего уровня схож с блоком then:

foreach (....)
{
  if (firstTime)
  {
    ....
    gridValueY = verticalStartSpace;
  }
  else
  {
    ....
    gridValueY = verticalStartSpace;
  }
  ....
  gridValueY = boundHeight - (gridValueY + gridValue);   // <=
  ....
}

В данном случае значение gridValueY тоже перезаписывается сразу после if, однако старое значение при этом используется.

Неправильно расставленные приоритеты

Issue 5

internal async Task<bool> StartResizeColumn(....)
{
  ....
  // In case resize mode is column, we have to find any column right
  // of the current one that can also be resized and is not hidden.

  var nextResizableColumn = _columns.Skip(_....) + 1)
                                    .FirstOrDefault(c =>
                                        c.Resizable ?? true && !c.Hidden); // <=
  ....
}

Предупреждение PVS-Studio: V3177 The 'true' literal belongs to the '&&' operator with a higher priority. It is possible the literal was intended to belong to '??' operator instead. DataGridColumnResizeService.cs 52

Рассмотрим выражение c.Resizable ?? true && !c.Hidden. Стоит сразу сказать, что приоритет '??' ниже, чем у '&&'. Исходя из этого, если c.Resizablenull, то оператор null-coalescing вернёт результат операции true && !c.Hidden. Операция "&&" с литералом true бессмысленна. Скорее всего, правильный вариант выглядит так:

(c.Resizable ?? true) && !c.Hidden

Разница этих записей проявит себя, если значения c.Resizable и c.Hidden будут равны true.

Есть ещё один момент, указывающий на то, что была допущена ошибка. Строчкой выше написан комментарий о том, что столбец должен иметь возможность изменения размера (информация об этом содержится в c.Resizable) и не быть скрытым (это можно узнать из c.Hidden). Если c.Resizabletrue, то логика работы будет нарушена, так как значение c.Hidden никак не повлияет на результат.

Подводя итог

Как мне кажется, каждое из разобранных срабатываний может указывать на ошибку, меняющую логику работы программы. В одном из случаев это удалось доказать. Перед написанием статьи я создал issue на GitHub. Однако, ответ от разработчиков пока не был получен. Надеюсь, что они рассмотрят перечисленные проблемы и смогут повысить качество кода своего проекта :).

Можно попробовать использованный в статье анализатор, скачав его отсюда. Кстати, недавно в PVS-Studio появилась возможность анализа блоков C# кода в Blazor-компонентах. Некоторые найденные в таких компонентах ошибки описаны в этой статье.

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


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

Следующие комментарии next comments
close comment form