# Обзор подозрительных мест в исходном коде MassTransit

MassTransit — Open Source платформа распределённых приложений для \.NET\. В этой статье мы расскажем о проблемных местах в коде проекта\. С поиском таких мест нам поможет статический анализатор\. Приятного чтения :\)\.

![1061_MassTransit_ru/image1.png](https://import.viva64.com/docx/blog/1061_MassTransit_ru/image1.png)

## Введение

Так как MassTransit предоставляет публичное API, качество кода такого проекта имеет особенно важную роль\. Неожиданное поведение данного продукта может негативно сказаться не только на нём самом, но и на использующих его приложения\. Это одна из причин, по которой мы решили проверить MassTransit\. Для анализа был взят исходный код, соответствующий [версии v8\.0\.15](https://github.com/MassTransit/MassTransit/tree/v8.0.15)\.

Не будем томить, приступим к разбору наиболее интересных фрагментов кода\.

## Подозрительное условие

**Issue 1**

```cpp
public async Task Send(OutboxConsumeContext<TMessage> context)
{
  ....

  if (   !context.ReceiveContext.IsDelivered       // <=
      && !context.ReceiveContext.IsDelivered)      // <=
  {
    await context.NotifyConsumed(context,
                                 timer.Elapsed,
                                 _options.ConsumerType)
                 .ConfigureAwait(false);
  }
  ....
}
```

Предупреждение PVS\-Studio: [V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions '\!context\.ReceiveContext\.IsDelivered' to the left and to the right of the '&&' operator\. OutboxMessagePipe\.cs 57

Обратите внимание на условие _if_\. Выглядит странно, ведь проверяется значение одного и того же свойства\. Очевидно, что подобная проверка не имеет смысла\. Однако если посмотреть свойства, к которым можно обратиться у объекта _context\.ReceiveContext_, то можно найти _IsFaulted_\. Возможно, именно оно должно фигурировать в одном из случаев\.

**Issue 2, 3**

```cpp
public static IEnumerable<Assembly> FindAssemblies(....)
{
  var assemblyPath = AppDomain.CurrentDomain.BaseDirectory;
  var binPath = string.Empty;                                           // <=

  if (string.IsNullOrEmpty(binPath))                                    // <=
    return FindAssemblies(assemblyPath,loadFailure, includeExeFiles, filter);

  if (Path.IsPathRooted(binPath))
    return FindAssemblies(binPath, loadFailure, includeExeFiles, filter);

  string[] binPaths = binPath.Split(';');
  return binPaths.SelectMany(bin =>
  {
      var path = Path.Combine(assemblyPath, bin);
      return FindAssemblies(path, loadFailure, includeExeFiles, filter);
  });
}
```

Здесь PVS\-Studio выдаёт сразу два предупреждения:

* [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'string\.IsNullOrEmpty\(binPath\)' is always true\. AssemblyFinder\.cs 23;
* [V3142](https://pvs-studio.ru/ru/docs/warnings/v3142/) Unreachable code detected\. It is possible that an error is present\. AssemblyFinder\.cs 26\.

Анализатор сообщает нам, что в данном коде обнаружено всегда истинное условие, а также недостижимый код\. Давайте разберёмся, как же так получилось\. 

В переменную _binPat_ присваивается пустая строка\. Сразу за присваиванием проверяется, что значение этой переменной равно _null_ или пустой строке\. Результат такой проверки — всегда истина\. Получается, что выполнение метода всегда заканчивается на этом условии, так как в блоке then возвращается значение\.

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

**Issue 4**

```cpp
public override Point? Read(ref Utf8JsonReader reader,
                            Type typeToConvert,
                            JsonSerializerOptions options)
{
  if (reader.TokenType != JsonTokenType.StartObject) // <=
    throw new JsonException("....");

  var originalDepth = reader.CurrentDepth;

  if (reader.TokenType == JsonTokenType.Null)        // <=
  {
    reader.Read();
    return null;
  }

  reader.Read();

  var x = double.NaN;
  var y = double.NaN;

  while (reader.TokenType == JsonTokenType.PropertyName)
  {
    ....
  }

  ....
}
```

Предупреждение PVS\-Studio: [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'reader\.TokenType \=\= JsonTokenType\.Null' is always false\. DoubleActivity\_Specs\.cs 55

Рассмотрим первое условие метода _Read_\. В нём производится проверка значения _reader\.TokenType_\. Если оно не соответствует _JsonTokenType\.StartObject_, то будет выброшено исключение\. Из этого следует, что после условия и до следующего вызова метода _Read_, _reader\.TokenType_ будет иметь значение _JsonTokenType\.StartObject_\. Получается, что в блок then второго _if_ поток выполнения не зайдёт никогда\. Возможно, после первого _if_ был пропущен вызов _Read_\.

## Потенциальное обращение по отрицательному индексу

**Issue 5**

```cpp
private static void
        ReturnClosureTypeToParamTypesToPool(Type[] closurePlusParamTypes)
{
  var paramCount = closurePlusParamTypes.Length - 1;                  // <=
  if (paramCount != 0 && paramCount < 8)
    Interlocked.Exchange(ref _closureTypePlusParamTypesPool[paramCount],
                         closurePlusParamTypes);
}
```

Предупреждение PVS\-Studio: [V3106](https://pvs-studio.ru/ru/docs/warnings/v3106/) Possible negative index value\. The value of 'paramCount' index could reach \-1\. ExpressionCompiler\.cs 555

Обратите внимание на переменную _paramCount_\. Её значение — размер массива, переданного в качестве аргумента, уменьшенный на 1\. Дальше это значение используется в качестве индекса\. Однако проверка перед использованием не гарантирует, что индекс не отрицательный\. В случае обращения по индексу с отрицательным значением будет выброшено исключение\. 

Возможно, разработчики уверены в том, что _closurePlusParamTypes_ всегда содержит хотя бы один элемент\. Может быть, в данный момент это действительно так\. Но нет гарантий того, что в будущем в данный метод не будет передана пустая коллекция\. 

**Issue 6**

```cpp
public static bool TryEmit(....)
{
  ....
  if ((parent & ParentFlags.InlinedLambdaInvoke) != 0)
  {
    var index = closure.GetLabelOrInvokeIndex(gt.Target); // <=
    var invokeIndex = closure.Labels
                             .Items[index]                // <=
                             .InlinedLambdaInvokeIndex;
    ....
  }
  ....
}
```

Предупреждение PVS\-Studio: [V3106](https://pvs-studio.ru/ru/docs/warnings/v3106/) Possible negative index value\. The value of 'index' index could reach \-1\. ExpressionCompiler\.cs 1977

Анализатор указывает на потенциальное обращение по отрицательному индексу\. Переменная _index_ содержит значение, использующееся в качестве индекса\. Оно будет получено в результате выполнения метода _GetLabelOrInvokeIndex_\. Рассмотрим определение этого метода:

```cpp
public short GetLabelOrInvokeIndex(object labelTarget)
{
  var count = Labels.Count;
  var items = Labels.Items;
  for (short i = 0; i < count; ++i)
    if (items[i].Target == labelTarget)
        return i;
  return -1;
}
```

Здесь мы видим, что один из вариантов возвращаемого значения — \-1\. Нетрудно догадаться, что такой сценарий не совсем желателен, так как будет выброшено исключение\.

Стоит отметить, что при работе с возвращаемым значением _GetLabelOrInvokeIndex_ в ряде других мест присутствует проверка на то, что оно не равно \-1\. Одна их них:

```cpp
public short AddInlinedLambdaInvoke(InvocationExpression e)
{
  var index = GetLabelOrInvokeIndex(e);

  if (index == -1)                                         // <=
  {
    ref var label = ref Labels.PushSlot();
    label.Target = e;
    index = (short)(Labels.Count - 1);
  }

  return index;
}
```

## Неиспользуемый параметр

**Issue 7**

```cpp
[Serializable]
public class BusHostInfo : HostInfo
{
  public BusHostInfo()
  {
  }

  public BusHostInfo(bool initialize)
  {
    FrameworkVersion = Environment.Version.ToString();
    OperatingSystemVersion = Environment.OSVersion.ToString();
    var entryAssembly =   System.Reflection.Assembly.GetEntryAssembly()
                       ?? System.Reflection.Assembly.GetCallingAssembly();

    MachineName = Environment.MachineName;

    MassTransitVersion = typeof(HostInfo).GetTypeInfo()
                                         .Assembly
                                         .GetName()
                                         .Version
                                         ?.ToString();

    try
    {
      using var currentProcess = Process.GetCurrentProcess();
      ProcessId = currentProcess.Id;
      ProcessName = currentProcess.ProcessName;
      if ("dotnet".Equals(ProcessName, StringComparison.OrdinalIgnoreCase))
        ProcessName = GetUsefulProcessName(ProcessName);
    }
    catch (PlatformNotSupportedException)
    {
      ProcessId = 0;
      ProcessName = GetUsefulProcessName("UWP");
    }

    var assemblyName = entryAssembly.GetName();
    Assembly = assemblyName.Name;
    AssemblyVersion = assemblyName.Version?.ToString() ?? "Unknown";
  }

  ....
}
```

Предупреждение PVS\-Studio: [V3117](https://pvs-studio.ru/ru/docs/warnings/v3117/) Constructor parameter 'initialize' is not used\. BusHostInfo\.cs 17

Параметр одной из перегрузок конструктора класса _BusHostInfo_ не был использован\. Стоит отметить, что у данного класса есть публичный конструктор без параметров, который вообще ничего не делает\. В итоге получается следующая ситуация: если использовать конструктор с параметром, то вне зависимости от его значения инициализация производится\. Если же воспользоваться конструктором, который не принимает параметров, то инициализация отсутствует\. Такое поведение выглядит весьма странно\.

Возможно, раньше параметр влиял на логику работы, однако перестал после рефакторинга\. Разработчик мог намерено не удалять неиспользуемый параметр, чтобы избежать ошибок у пользователей\. Ошибки могли возникнуть из\-за того, что класс _BusHostInfo_ публичный, как и рассматриваемый конструктор\. Это значит, что он доступен пользователям напрямую\. Если поменять сигнатуру метода \(убрать параметр\), то в местах вызова данного конструктора возникнут ошибки из\-за отсутствия нужной перегрузки\. Если данное предположение верно, то хотелось бы видеть комментарий, поясняющий этот момент\.

## Заключение

Можно сказать, что код проекта оказался чистым\. Во многом это заслуга разработчиков, однако стоит учесть, что они уже использовали другой статический анализатор\. Это стало понятно, исходя из комментариев для подавления его срабатываний\. Даже при этом условии удалось найти ряд подозрительных мест\. Возможно, анализатором пользовались ранее, а сейчас перестали\.

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

Если у вас появилось желание проверить исходный код интересующего проекта, то вы можете [бесплатно попробовать PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/try-free/)\. Удачного использования\!