Jellyfin — это бесплатный медиасервер с открытым исходным кодом, является альтернативой Emby и Plex. В этой статье мы рассмотрим диагностику, у которой было больше всего срабатываний в коде Jellyfin. И ещё несколько ошибок проекта.

Немного о проекте

Jellyfin создан на основе релиза Emby 3.5.2 и перенесён на платформу .NET Core для обеспечения полной кроссплатформенной поддержки.

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

Проект представляет собой продолжение разработки Emby, который с декабря 2018 года не является мультимедийной системой с открытым исходным кодом. Чтобы попробовать Jellyfin и насладиться всеми его функциями, разработчик рекомендует начать со свежей базы данных и сканирования библиотеки.

Медиасервер Jellyfin пользуется популярностью среди пользователей, но ранее не был замечен PVS-Studio. Хотя его прародитель — проект Emby — был проверен нашим анализатором. Наконец, анализатор PVS-Studio добрался и до Jellyfin. Поэтому перейдём к ошибкам проекта. Для проверки использован исходный код Jellyfin, который доступен для скачивания на GitHub.

Как работает диагностика V3022

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

В основе диагностики V3022 лежит анализ потоков данных (data flow анализ). Анализ потоков данных — это методика, позволяющая статическому анализатору делать предположения о значениях переменных и выражений в различных частях исходного кода. Под предполагаемыми значениями понимаются какие-то конкретные значения, диапазоны значений или множество возможных значений. Дополнительно собирается и обрабатывается информация о том, освобождена память по указателю или нет, каковы размеры массивов и так далее.

Предположения о значениях строятся на основе анализа движения значений переменных по графу выполнения программы (control flow graph). В большинстве случаев анализатор не может знать точного значения переменной или выражения. Но он может, используя прямые и косвенные ограничения, накладываемые на рассматриваемые выражения по ходу обхода графа выполнения, делать предположения о диапазонах или множествах значений, которые эти выражения могут принимать в различных точках графа.

Все анализаторы PVS-Studio используют data flow анализ для уточнения работы своих диагностических правил. Это требуется в случаях, когда для принятия решения об опасности рассматриваемого кода недостаточно информации, доступной только из AST или семантической структуры этого кода.

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

Ошибки проекта

Ошибки в условии

Как было написано выше, больше всего срабатываний пришлось на диагностику V3022. Им и будет посвящена статья. Но я добавил ещё парочку срабатываний, чтобы показать, что в проекте есть разные виды ошибок.

Вариантов ошибок в срабатывании V3022 множество: взаимоисключающие проверки, ошибка в условии, всегда истинное/ложное условие. Но относится всё это к ошибкам в условиях.

Issue 1

private void ApplyTranscodingConditions(....)
{
  ....

  if (condition.Condition == 
                     ProfileConditionType.GreaterThanEqual)  // <=
  {
    continue;
  }

  switch (condition.Property)
  {
    case ProfileConditionValue.AudioBitrate:
    {
      ....
      else if (condition.Condition ==
                              ProfileConditionType.GreaterThanEqual)  // <=
      {
        ....                        
      }

      break;
    }

    case ProfileConditionValue.AudioSampleRate:
    {
      ....
      else if (condition.Condition ==
                              ProfileConditionType.GreaterThanEqual)  // <=
      {
        ....                        
      }

      break;
      ....
    }
  }
}

Предупреждение PVS-Studio: V3022 Expression 'condition.Condition == ProfileConditionType.GreaterThanEqual' is always false. StreamBuilder.cs 2193

Это самая масштабная ошибка проекта. Автор повторил её в 10 блоках case! В примере приведено несколько из них — ошибка одинаковая во всех блоках.

Всё из-за условия в начале программы. Если выражение condition.Condition == ProfileConditionType.GreaterThanEqual принимает значение true, то программа попадает внутрь условия и перейдёт к следующей итерации цикла, так как использован оператор continue. Следовательно, ниже, в блоках case, никогда не будет true. Код внутри них не выполнится. Автор забыл о первой проверке и продолжил копировать ошибку в 10 блоков case. Подобное приведёт к неожиданной работе программы и потере её функционала.

Issue 2

private static bool AllowBoxSetCollapsing(InternalItemsQuery request)
{
  ....
  if (request.IsResumable.HasValue)
  {
    return false;
  }

  if (request.IsFolder.HasValue)
  {
    return false;
  }

  if (request.Genres.Count > 0)
  {
    return false;
  }

  if (request.GenreIds.Count > 0)  // <=
  {
    return false;
  }

  ....

  if (request.GenreIds.Count > 0)  // <=
  {
    return false;
  }

  ....
}

Предупреждение PVS-Studio: V3022 Expression 'request.GenreIds.Count > 0' is always false. Folder.cs 1243

Анализатор сообщает о лишним условии. Автор проверил переменную request.GenreIds.Count, а потом ниже сделал это ещё раз. Скорее всего, он хотел проверить другую переменную и не заметил ошибки.

Issue 3

private IReadOnlyList<BaseItem> GetItemsForLatestItems(....)
{
  ....
  if (parents.Count == 0)  // <=
  {
    parents = _libraryManager.GetUserRootFolder().GetChildren(user, true)
      .Where(i => i is Folder)
      .Where(i => !user.GetPreferenceValues<Guid>
                        PreferenceKind.LatestItemExcludes)
      .Contains(i.Id))
      .ToList();
  }

  if (parents.Count == 0)  // <=
  {
    return Array.Empty<BaseItem>();
  }
  ....
  if (parents.Count == 0)  // <=
  {
    return _libraryManager.GetItemList(query, false);
  }
}

Предупреждение PVS-Studio: V3022 Expression 'parents.Count == 0' is always false. UserViewManager.cs 372

Здесь автор проверяет количество значений parents.Count. Если оно равно 0, то он заполняет parents значениями. Пока всё логично. Затем он второй раз проверяет переменную parents.Count, вдруг не заполнилась. Снова логично. Потом происходит третья проверка, чтобы точно не ошибиться. Но, видимо, автор не учёл, что после второй проверки, при значении parents.Count равном 0, программа вернёт значение Array.Empty<BaseItem>() и выйдет из этого метода. Поэтому программа никогда не дойдёт до третьего условия, оно здесь лишнее.

Issue 4

public override string CreatePresentationUniqueKey()
{
  if (IndexNumber.HasValue)
  {
    var series = Series;
    if (series is not null)
    {
      return series.PresentationUniqueKey + "-" + 
               (IndexNumber ?? 0).ToString("000", CultureInfo.InvariantCulture);
    }
  }

  return base.CreatePresentationUniqueKey();
}

Предупреждение PVS-Studio: V3022 Expression 'IndexNumber' is always not null. The operator '??' is excessive. Season.cs 135

В этом фрагменте сначала происходит проверка значения переменной IndexNumber, а затем снова проверка на null. Но проверка избыточна, так как переменная имеет значение, если попала в этот if.

Похожая ошибка, снова с оператором ??.

Issue 5

private int? GetOutputVideoCodecLevel(StreamState state)
{
  string levelString = string.Empty;
  if (EncodingHelper.IsCopyCodec(state.OutputVideoCodec)
       && state.VideoStream is not null
       && state.VideoStream.Level.HasValue)
  {
    levelString = state.VideoStream.Level.Value.
                        ToString(CultureInfo.InvariantCulture) ?? string.Empty;
  }
}

Предупреждение PVS-Studio: V3022 Expression 'state.VideoStream.Level.Value.ToString(CultureInfo.InvariantCulture)' is always not null. The operator '??' is excessive. DynamicHlsHelper.cs 619

Анализатор сообщает об ошибке в использовании оператора ??. Автор рассчитывал, что он избавит от значения null, так как этот оператор сравнивает значения только с null. Но ни метод ToString(), ни параметр string.Empty не возвращают null, поэтому проверка лишняя.

Посмотрим метод ToString(), чтобы убедиться, что он не возвращает null:

public virtual string ToString()
{
  return GetType().ToString();
}

Посмотрим ниже:

public override string ToString()
{
  return "Type: " + Name;
}

По этому коду видно, что возвращаемое значение — строка. Этот метод не вернёт null.

Issue 6

public AggregateFolder CreateRootFolder()
{
  ....
  var path = Path.Combine(_configurationManager.ApplicationPaths.DataPath,
                          "playlists");

  ....

  Folder folder = new PlaylistsFolder
  {
    Path = path
  };

  if (folder.Id.IsEmpty())
  {
    if (string.IsNullOrEmpty(folder.Path))
    {
      ....
    }
    ....
  }
  ....
}

Предупреждение PVS-Studio: V3022 Expression 'string.IsNullOrEmpty(folder.Path)' is always false. LibraryManager.cs 754

Во фрагменте метод Combine() всегда возвращает string. Если какой-то из путей будет null, он выдаст ArgumentNullException и ничего не вернёт. Это можно увидеть ниже:

public static string Combine(string path1, string path2)
{
  if (path1 == null || path2 == null)
  {
    throw new ArgumentNullException((path1 == null) ? "path1" : "path2");
  }

  CheckInvalidPathChars(path1);
  CheckInvalidPathChars(path2);
  return CombineNoChecks(path1, path2);
}

Поэтому метод Combine() всегда вернёт string. Его значение передаётся в переменную Path. Следовательно, Path не может быть null, поэтому эта проверка лишняя.

Issue 7

private async Task<ActionResult> GetMasterPlaylistInternal(....)
{
  ....
  if (EnableAdaptiveBitrateStreaming(state, ....)
  {
    ....
  }
}

Предупреждение PVS-Studio: V3022 Expression is always false. DynamicHlsHelper.cs 268

Анализатор сообщает, что выражение будет всегда false. Чтобы понять, в чём ошибка, нужно заглянуть в метод EnableAdaptiveBitrateStreaming(). Его код представлен ниже:

private bool EnableAdaptiveBitrateStreaming(StreamState state,....)
{
  if (....)
  {
    return false;
  }
  
  if (....)
  {
    return false;
  }

  if (....)
  {
    return false;
  }
  
  if (....)
  {
    return false;
  }

  if (....)
  {
    return false;
  }

  if (....)
  {
    return false;
  }
  
  // Having problems in android
  return false;
  // return state.VideoRequest.VideoBitRate.HasValue;
}

А вот и ошибка! Метод всегда возвращает false. Наверно, автор на время фикса решил выключить функционал, который приводил к багам, а потом забыл его раскомментировать. Судя по комментарию, код не работал на Android.

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

Issue 8

private string GetCommandLineArgs(MediaSourceInfo mediaSource, 
string inputTempFile, string targetFile)
{
  ....
  if (EncodeVideo(mediaSource))  // <=
  {
    const int MaxBitrate = 25000000;
    videoArgs = string.Format(CultureInfo.InvariantCulture,
               "-codec:v:0 libx264 -force_key_frames \
               "expr:gte(t,n_forced*5)\" {0} 
               -pix_fmt yuv420p -preset superfast -crf 23
               -b:v {1} -maxrate {1} -bufsize ({1}*2) -profile:v high 
               -level 41",GetOutputSizeParam(),MaxBitrate);
  }
  ....
}

Предупреждение PVS-Studio: V3022 Expression 'EncodeVideo(mediaSource)' is always false. EncodedRecorder.cs 128

Такая же ошибка, что и выше. По предыдущему опыту поищем ошибку в вызываемом методе EncodeVideo():

private static bool EncodeVideo(MediaSourceInfo mediaSource)
{
  return false;
}

Метод, который всегда возвращает false?! Зачем тогда был реализован весь функционал в if()? В случаях намеренного отключения функционала нужно быть осторожными, чтобы не забыть о нём. В таких ситуациях помогает анализатор PVS-Studio. Он легко нашёл две ошибки, где автор мог упустить функционал из-за забывчивости.

Ошибки с NRE

Ошибки при неверном использовании переменных без проверки на null, из-за которых может возникнуть NullReferenceException.

Issue 9

protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
{
  ....
  if (authorizationInfo.IsApiKey || 
      authorizationInfo.User.HasPermission(PermissionKind.IsAdministrator))
  {
    ....
  }
  
  var claims = new[]
  {
    new Claim(ClaimTypes.Name,
              authorizationInfo.User?.Username ?? string.Empty),
    ....
  };
  ....
}

Предупреждение PVS-Studio: V3095 The 'authorizationInfo.User' object was used before it was verified against null. Check lines: 53, 60. CustomAuthenticationHandler.cs 53

Автор упустил ошибку, которая может привести к NRE. Здесь разработчик использует вызов метода authorizationInfo.User.HasPermission(PermissionKind.IsAdministrator). Если переменная authorizationInfo.User имеет значение null, то при использовании её метода будет NRE. Да, автор проверяет эту переменную, но не в том месте — ошибка NRE могла уже произойти.

Issue 10

private void BuildStreamVideoItem(....)
{
  ....
  if (maxBitrateSetting.HasValue)
  {
    ....
    playlistItem.VideoBitrate = Math.Max(Math.Min(availableBitrateForVideo, 
                                currentValue), 64_000);
  }
  _logger.LogDebug(
      ....
      playlistItem?.PlayMethod,
      ....);
}

Предупреждение PVS-Studio: V3095 The 'playlistItem' object was used before it was verified against null. Check lines: 1077, 1084. StreamBuilder.cs 1077

Такая же ошибка, в которой разработчик сначала использует переменную playlistItem, а потом проверяет её на null. Эта ошибка тоже может привести к NRE.

Ошибка copy-paste

Ошибка copy-paste, которую разработчик не заметил из-за невнимательности. В итоге программа работает не так, как ожидалось.

Issue 11

private void FetchBdInfo(....)
{
  ....
  if (blurayVideoStream is not null && ffmpegVideoStream is not null)
  {
    ....
    blurayVideoStream.Width = blurayVideoStream.Width.GetValueOrDefault()
          == 0 ? ffmpegVideoStream.Width : blurayVideoStream.Width;

    blurayVideoStream.Height = blurayVideoStream.Height.GetValueOrDefault()
          == 0 ? ffmpegVideoStream.Width : blurayVideoStream.Height;
    ....
  }
}

Предупреждение PVS-Studio: V3127 Two similar code fragments were found. Perhaps, this is a typo and 'Height' variable should be used instead of 'Width' FFProbeVideoInfo.cs 357

Анализатор PVS-Studio выдал ошибку об одинаковых фрагментах кода. Возникла она, скорее всего, из-за невнимательности или копирования. Если сравнивать со строкой выше, то разработчик должен был использовать переменную ffmpegVideoStream.Height, но ошибся и скопировал из предыдущей строки ffmpegVideoStream.Width. Поэтому результат будет не тот, который задумывал автор. Ошибка copy-paste уже рассматривалась в другой статье, она встречается довольно часто у разработчиков.

Ошибка в форматировании строки

Эту ошибку автор не заметил. Но она может привести к потере информации из оповещения.

Issue 12

public static byte MaskToCidr(IPAddress mask)
{
  ....
  if (!mask.TryWriteBytes(bytes, out var bytesWritten))
  {
    Console.WriteLine("Unable to write address bytes, 
                       only ${bytesWritten} bytes written.");
  }
  ....
}

Предупреждение PVS-Studio: V3138 String literal contains potential interpolated expression. Consider inspecting: bytesWritten. NetworkUtils.cs 105

Тут, по логике кода, разработчик выводит предупреждение о количестве байтов (bytesWritten). Чтобы вывести значение этой переменной, нужно взять выражение в кавычки. Без кавычек компилятор принимает эту переменную как строку. Поэтому при выполнении условия разработчик увидит просто строку с названием переменной и не получит нужную информацию из этого предупреждения.

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

Заключение

Медиасервер Jellyfin популярен среди пользователей. Благодаря ему можно организовать трансляцию медиаданных на сетевые устройства. Сервер часто используют для просмотра фильмов.

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

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

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

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Nikita Sviridov. Ode to one diagnostic rule and to couple more, or Jellyfin check.

Ода одной диагностике и ещё парочке, или проверка Jellyfin