Хабрахабр

Ищем ошибки в исходном коде Amazon Web Services SDK для .NET

Picture 1

Приветствую всех любителей покритиковать чужой код. 🙂 Сегодня в нашей лаборатории новый материал для исследования — исходный код проекта AWS SDK для .NET. В своё время мы писали статью о проверке AWS SDK для C++. Тогда не нашлось ничего особо интересного. Посмотрим, чем нас порадует .NET версия AWS SDK. Хорошая возможность в очередной раз продемонстрировать возможности анализатора PVS-Studio, а также сделать мир немного совершеннее.
Amazon Web Services (AWS) SDK для .NET — это набор инструментов разработчика, предназначенный для создания приложений на основе .NET в инфраструктуре AWS и позволяющий значительно упростить процесс написания кода. SDK включает в себя наборы API .NET для различных сервисов AWS, таких как Amazon S3, Amazon EC2, DynamoDB и других. Исходный код SDK размещён на GitHub.

Статья получилась небольшая — на 512 тысяч строк кода нашлась всего пара ошибок. Как я уже говорил, в своё время мы писали статью о проверке AWS SDK для C++. Незначительная часть кода (200 тысяч строк в 664 cs-файлах) приходится на тесты, их я не рассматривал. В этот раз мы имеем дело с гораздо большим объёмом кода, включающим в себя около 34 тысяч cs-файлов, а общее число строк кода (без учета пустых) составляет впечатляющие 5 миллионов.

NET версии SDK приблизительно такое же, как у C++ (две ошибки на 512 KLOC), то мы должны получить примерно в 10 раз большее число ошибок. Если качество кода . Вместо этого предлагаю сразу перейти к результатам. Это, конечно, очень неточная методика подсчёта, не учитывающая языковые особенности и ещё множество других факторов, но не думаю, что читателю сейчас хочется углубляться в скучные рассуждения.

27. Проверка производилась при помощи PVS-Studio версии 6. NET анализатору удалось обнаружить около 40 ошибок, о которых стоило бы рассказать. Невероятно, но в коде AWS SDK для . Отличный результат! Это говорит не только о высоком качестве кода SDK (примерно 4 ошибки на 512 KLOC), но и о сопоставимом качестве работы C# анализатора PVS-Studio в сравнении с С++.

NET — большие молодцы. Авторы AWS SDK для . Это может служить хорошим примером для других команд. От проекта к проекту они демонстрируют потрясающее качество кода. 🙂 Мы уже взаимодействуем с командой Lumberyard из Amazon по вопросам использования PVS-Studio. Но, конечно, я не был бы разработчиком статического анализатора, если бы не вставил свои 5 копеек. NET вообще никогда не слышала о PVS-Studio. Но так как это очень большая компания с кучей подразделений по миру, то весьма вероятно, что команда AWS SDK для . Тем не менее, команда, как минимум, использует анализатор, встроенный в Visual Studio. Во всяком случае, в коде SDK я не нашёл никаких признаков использования нашего анализатора, хотя это ни о чем и не говорит. Это хорошо, но проверки кода можно усилить :).

Как итог, мне все же удалось найти несколько ошибок в коде SDK и, наконец, пришло время ими поделиться.

Ошибка в логике

Perhaps this is a mistake. Предупреждение PVS-Studio: V3008 [CWE-563] The 'this.linker.s3.region' variable is assigned values twice successively. AWSSDK. Check lines: 116, 114. Net45 S3Link.cs 116 DynamoDBv2.

public string Region set { if (String.IsNullOrEmpty(value)) { this.linker.s3.region = "us-east-1"; } this.linker.s3.region = value; } }

Анализатор предупреждает о повторном присваивании значения одной и той же переменной. Из кода становится понятно, что это связано с ошибкой, которая нарушает логику работы: значение переменной this.linker.s3.region всегда будет равно value, независимо от условия if (String.IsNullOrEmpty(value)). В теле блока if был пропущен return. Код необходимо исправить следующим образом:

public string Region
{ get { .... } set { if (String.IsNullOrEmpty(value)) { this.linker.s3.region = "us-east-1"; return; } this.linker.s3.region = value; } }

Бесконечная рекурсия

AWSSDK. Предупреждение PVS-Studio: V3110 [CWE-674] Possible infinite recursion inside 'OnFailure' property. Net45 ResizeJobFlowStep.cs 171 ElasticMapReduce.

OnFailure? onFailure = null; public OnFailure? OnFailure
{ get { return this.OnFailure; } // <= set { this.onFailure = value; }
}

Классический пример опечатки, которая приводит к возникновению бесконечной рекурсии в методе доступа get свойства OnFailure. Вместо возврата значения приватного поля onFailure обращаются к свойству OnFailure. Исправленный вариант:

public OnFailure? OnFailure
{ get { return this.onFailure; } set { this.onFailure = value; }
}

Вы спросите: «Как это работало?» Пока — никак. Свойство нигде не используется, но это временное явление. В один прекрасный момент кто-то начнёт его использовать и обязательно получит непредвиденный результат. Для предотвращения таких опечаток рекомендуется не использовать идентификаторы, различающиеся только регистром первой буквы.

С точки зрения компилятора это вполне допустимо, но затрудняет восприятие кода человеком. Ещё одно замечание к данной конструкции — использование идентификатора, полностью совпадающего с именем типа OnFailure.

Другая подобная ошибка:

AWSSDK. Предупреждение PVS-Studio: V3110 [CWE-674] Possible infinite recursion inside 'SSES3' property. Net45 InventoryEncryption.cs 37 S3.

private SSES3 sSES3; public SSES3 SSES3
{ get { return this.SSES3; } set { this.SSES3 = value; }
}

Ситуация идентична описанной выше. Только здесь бесконечная рекурсия возникнет при обращении к свойству SSES3 как на чтение, так и на запись. Исправленный вариант:

public SSES3 SSES3
{ get { return this.sSES3; } set { this.sSES3 = value; }
}

Тест на внимательность

Посмотрите на то, как код выглядит в редакторе Visual Studio, и попробуйте найти ошибку. Далее следует задание от программиста, увлечённого использованием методики Copy-Paste.

Picture 3

Предупреждение PVS-Studio: V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 91, 95. AWSSDK.AppSync.Net45 CreateApiKeyResponseUnmarshaller.cs 91

Теперь видно, что идентичные проверки следуют одна за другой: Я сократил тело метода UnmarshallException, убрав всё лишнее.

public override AmazonServiceException UnmarshallException(....)
{ .... if (errorResponse.Code != null && errorResponse.Code.Equals("LimitExceededException")) { return new LimitExceededException(errorResponse.Message, innerException, errorResponse.Type, errorResponse.Code, errorResponse.RequestId, statusCode); } if (errorResponse.Code != null && errorResponse.Code.Equals("LimitExceededException")) { return new LimitExceededException(errorResponse.Message, innerException, errorResponse.Type, errorResponse.Code, errorResponse.RequestId, statusCode); } ....
}

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

В коде есть ещё несколько подобных ошибок.

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

  • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 75, 79. AWSSDK.CloudDirectory.Net45 CreateSchemaResponseUnmarshaller.cs 75
  • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 105, 109. AWSSDK.CloudDirectory.Net45 GetSchemaAsJsonResponseUnmarshaller.cs 105
  • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 201, 205. AWSSDK.CodeCommit.Net45 PostCommentForPullRequestResponseUnmarshaller.cs 201
  • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 101, 105. AWSSDK.CognitoIdentityProvider.Net45 VerifySoftwareTokenResponseUnmarshaller.cs 101
  • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 72, 76. AWSSDK.Glue.Net45 UpdateConnectionResponseUnmarshaller.cs 72
  • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 123, 127. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 123
  • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 167, 171. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 167
  • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 127, 131. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 127
  • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 171, 175. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 171
  • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 99, 103. AWSSDK.Rekognition.Net45 RecognizeCelebritiesResponseUnmarshaller.cs 99

Что ты такое?

Consider checking the first actual argument of the 'Contains' method. Предупреждение PVS-Studio: V3062 An object 'attributeName' is used as an argument to its own method. MobileAnalytics. AWSSDK. Net45 CustomEvent.cs 261

/// <summary>
/// Dictionary that stores attribute for this event only.
/// </summary>
private Dictionary<string,string> _attributes = new Dictionary<string,string>(); /// <summary>
/// Gets the attribute.
/// </summary> /// <param name="attributeName">Attribute name.</param>
/// <returns>The attribute. Return null of attribute doesn't
/// exist.</returns>
public string GetAttribute(string attributeName)
{ if(string.IsNullOrEmpty(attributeName)) { throw new ArgumentNullException("attributeName"); } string ret = null; lock(_lock) { if(attributeName.Contains(attributeName)) // <= ret = _attributes[attributeName]; } return ret;
}

Анализатор обнаружил ошибку в методе GetAttribute: строку проверяют на то, что она содержит саму себя. Из описания к методу следует, что если имя атрибута (ключ attributeName) будет найдено (в словаре _attributes), то следует вернуть значение атрибута, иначе — null. В действительности же, так как условие attributeName.Contains(attributeName) всегда истинно, производится попытка возврата значения по ключу, который может быть не найден в словаре. Тогда вместо возврата null будет выброшено исключение KeyNotFoundException.

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

/// <summary>
/// Determines whether this instance has attribute the specified
/// attributeName.
/// </summary>
/// <param name="attributeName">Attribute name.</param>
/// <returns>Return true if the event has the attribute, else
/// false.</returns>
public bool HasAttribute(string attributeName)
{ if(string.IsNullOrEmpty(attributeName)) { throw new ArgumentNullException("attributeName"); } bool ret = false; lock(_lock) { ret = _attributes.ContainsKey(attributeName); } return ret;
}

Данный метод проверяет, существует ли имя атрибута (ключ attributeName) в словаре _attributes. Снова вернёмся к методу GetAttribute и исправим ошибку:

public string GetAttribute(string attributeName)
{ if(string.IsNullOrEmpty(attributeName)) { throw new ArgumentNullException("attributeName"); } string ret = null; lock(_lock) { if(_attributes.ContainsKey(attributeName)) ret = _attributes[attributeName]; } return ret;
}

Теперь метод делает именно то, что заявлено в описании.

Я заметил, что авторы используют lock при работе со словарем _attributes. И ещё одно небольшое замечание к данному фрагменту кода. Вместо Dictionary в данном случае, возможно, удобнее было бы использовать потокобезопасный вариант словаря — ConcurrentDictionary. Ясно, что это необходимо при многопоточном доступе, но конструкция lock достаточно медленна и громоздка. Хотя, возможно я не знаю о каких-то особенностях проекта. Тогда необходимость в lock отпадает.

Подозрительное поведение

IsNullOrEmpty(inferredIndexName). Предупреждение PVS-Studio: V3063 [CWE-571] A part of conditional expression is always true if it is evaluated: string. DynamoDBv2. AWSSDK. PCL ContextInternal.cs 802

private static string GetQueryIndexName(....)
{ .... string inferredIndexName = null; if (string.IsNullOrEmpty(specifiedIndexName) && indexNames.Count == 1) { inferredIndexName = indexNames[0]; } else if (indexNames.Contains(specifiedIndexName, StringComparer.Ordinal)) { inferredIndexName = specifiedIndexName; } else if (string.IsNullOrEmpty(inferredIndexName) && // <= indexNames.Count > 0) throw new InvalidOperationException("Local Secondary Index range key conditions are used but no index could be inferred from model. Specified index name = " + specifiedIndexName); ....
}

Анализатор насторожила проверка string.IsNullOrEmpty(inferredIndexName). Действительно, строке inferredIndexName присваивают null, далее значение этой переменной нигде не изменяется, а потом её зачем-то проверяют на null или пустую строку. Выглядит подозрительно. Давайте внимательно посмотрим на приведённый фрагмент кода. Я умышленно не стал его сокращать для лучшего понимания ситуации. Итак, в первом операторе if (а также в следующем), некоторым образом проверяют переменную specifiedIndexName. В зависимости от результата проверок, переменная inferredIndexName получает новое значение. Теперь обратим внимание на третий оператор if. Тело этого оператора (выброс исключения) будет выполнено в случае indexNames.Count > 0, так как первая часть условия — string.IsNullOrEmpty(inferredIndexName) — всегда истина. Возможно, просто перепутаны переменные specifiedIndexName и inferredIndexName. Или же третья проверка должна быть без else, представляя собой автономный оператор if:

if (string.IsNullOrEmpty(specifiedIndexName) && indexNames.Count == 1)
{ inferredIndexName = indexNames[0];
}
else if (indexNames.Contains(specifiedIndexName, StringComparer.Ordinal))
{ inferredIndexName = specifiedIndexName;
} if (string.IsNullOrEmpty(inferredIndexName) && indexNames.Count > 0) throw new InvalidOperationException(....);

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

NullReferenceException

Check lines: 228, 238. Предупреждение PVS-Studio: V3095 [CWE-476] The 'conditionValues' object was used before it was verified against null. Core. AWSSDK. Net45 JsonPolicyWriter.cs 228

private static void writeConditions(....)
{ .... foreach (....) { IList<string> conditionValues = keyEntry.Value; if (conditionValues.Count == 0) // <= continue; .... if (conditionValues != null && conditionValues.Count != 0) { .... } .... }
}

Классика жанра. Переменную conditionValues используют без предварительной проверки на равенство null. При этом далее в коде такая проверка выполняется, но уже поздно. 🙂

Код можно исправить следующим образом:

private static void writeConditions(....)
{ .... foreach (....) { IList<string> conditionValues = keyEntry.Value; if (conditionValues != null && conditionValues.Count == 0) continue; .... if (conditionValues != null && conditionValues.Count != 0) { .... } .... }
}

В коде было найдено ещё несколько подобных ошибок.

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

  • V3095 [CWE-476] The 'ts.Listeners' object was used before it was verified against null. Check lines: 140, 143. AWSSDK.Core.Net45 Logger.Diagnostic.cs 140
  • V3095 [CWE-476] The 'obj' object was used before it was verified against null. Check lines: 743, 745. AWSSDK.Core.Net45 JsonMapper.cs 743
  • V3095 [CWE-476] The 'multipartUploadMultipartUploadpartsList' object was used before it was verified against null. Check lines: 65, 67. AWSSDK.S3.Net45 CompleteMultipartUploadRequestMarshaller.cs 65

Следующее предупреждение очень похоже по смыслу, но ситуация обратна рассмотренной выше.

Check lines: 139, 127. Предупреждение PVS-Studio: V3125 [CWE-476] The 'state' object was used after it was verified against null. Core. AWSSDK. Net45 RefreshingAWSCredentials.cs 139

private void UpdateToGeneratedCredentials( CredentialsRefreshState state)
{ string errorMessage; if (ShouldUpdate) { .... if (state == null) errorMessage = "Unable to generate temporary credentials"; else .... throw new AmazonClientException(errorMessage); } state.Expiration -= PreemptExpiryTime; // <= ....
}

Один из фрагментов кода содержит проверку значения переменной state на равенство null. Далее по коду переменную state используют для того, чтобы отписаться от события PreemptExpiryTime, при этом проверка на равенство null уже не производится, и возможен выброс исключения NullReferenceException. Более безопасный вариант кода:

private void UpdateToGeneratedCredentials( CredentialsRefreshState state)
{ string errorMessage; if (ShouldUpdate) { .... if (state == null) errorMessage = "Unable to generate temporary credentials"; else .... throw new AmazonClientException(errorMessage); } if (state != null) state.Expiration -= PreemptExpiryTime; ....
}

В коде есть и другие подобные ошибки.

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

  • V3125 [CWE-476] The 'wrappedRequest.Content' object was used after it was verified against null. Check lines: 395, 383. AWSSDK.Core.Net45 HttpHandler.cs 395
  • V3125 [CWE-476] The 'datasetUpdates' object was used after it was verified against null. Check lines: 477, 437. AWSSDK.CognitoSync.Net45 Dataset.cs 477
  • V3125 [CWE-476] The 'cORSConfigurationCORSConfigurationcORSRulesListValue' object was used after it was verified against null. Check lines: 125, 111. AWSSDK.S3.Net45 PutCORSConfigurationRequestMarshaller.cs 125
  • V3125 [CWE-476] The 'lifecycleConfigurationLifecycleConfigurationrulesListValue' object was used after it was verified against null. Check lines: 157, 68. AWSSDK.S3.Net45 PutLifecycleConfigurationRequestMarshaller.cs 157
  • V3125 [CWE-476] The 'this.Key' object was used after it was verified against null. Check lines: 199, 183. AWSSDK.S3.Net45 S3PostUploadRequest.cs 199

Безальтернативная реальность

AWSSDK. Предупреждение PVS-Studio: V3009 [CWE-393] It's odd that this method always returns one and the same value of 'true'. Net45 Lexer.cs 651 Core.

private static bool State19 (....)
{ while (....) { switch (....) { case '"': .... return true; case '\\': .... return true; default: .... continue; } } return true;
}

Метод всегда возвращает true. Давайте посмотрим, насколько это критично для вызывающего кода. Я отследил использование метода State19. Он участвует в заполнении массива обработчиков fsm_handler_table наравне с другими подобными методами (их 28 с именами, соответственно, от State1 до State28). Здесь важно отметить, что помимо State19, для некоторых других обработчиков также были выданы предупреждения V3009 [CWE-393]. Это обработчики: State23, State26, State27, State28. Предупреждения, выданные анализатором для них:

  • V3009 [CWE-393] It's odd that this method always returns one and the same value of 'true'. AWSSDK.Core.Net45 Lexer.cs 752
  • V3009 [CWE-393] It's odd that this method always returns one and the same value of 'true'. AWSSDK.Core.Net45 Lexer.cs 810
  • V3009 [CWE-393] It's odd that this method always returns one and the same value of 'true'. AWSSDK.Core.Net45 Lexer.cs 822
  • V3009 [CWE-393] It's odd that this method always returns one and the same value of 'true'. AWSSDK.Core.Net45 Lexer.cs 834

Вот как выглядит объявление и инициализация массива обработчиков:

private static StateHandler[] fsm_handler_table;
....
private static void PopulateFsmTables ()
{ fsm_handler_table = new StateHandler[28] { State1, State2, .... State19, .... State23, .... State26, State27, State28
};

Для полноты картины — посмотрим код одного из обработчиков, к которому у анализатора не было претензий, например, State2:

private static bool State2 (....)
{ .... if (....) { return true; } switch (....) { .... default: return false; }
}

А вот так происходит вызов обработчиков:

public bool NextToken ()
{ .... while (true) { handler = fsm_handler_table[state - 1]; if (! handler (fsm_context)) // <= throw new JsonException (input_char); .... } ....
}

Как видим, исключение будет выброшено в случае возврата false. В нашем случае, для обработчиков State19, State23, State26, State27 и State28 этого не произойдёт никогда. Выглядит подозрительно. С другой стороны, целых пять обработчиков имеют схожее поведение (всегда вернут true), так что, возможно, это так было задумано и не является следствием опечатки.

Данная ситуация очень показательна в том плане, что статический анализатор часто способен лишь указать на подозрительную конструкцию. Зачем я на всем этом так подробно остановился? Код необходимо изучить разработчику. И даже человек (не машина), не обладающий достаточными знаниями о проекте, потратив время на изучение кода, всё равно не способен дать исчерпывающего ответа о наличии ошибки.

Бессмысленные проверки

AWSSDK. Предупреждение PVS-Studio: V3022 [CWE-571] Expression 'doLog' is always true. Net45 StoredProfileAWSCredentials.cs 235 Core.

private static bool ValidCredentialsExistInSharedFile(....)
{ .... var doLog = false; try { if (....) { return true; } else { doLog = true; } } catch (InvalidDataException) { doLog = true; } if (doLog) // <= { .... } ....
}

Обратите внимание на переменную doLog. После инициализации значением false, далее в коде эта переменная получит значение true во всех случаях. Таким образом, проверка if (doLog) всегда истинна. Возможно, ранее в данном методе существовала ветка, в которой переменной doLog не присваивалось никакого значения, тогда на момент проверки она могла содержать значение false, полученное при инициализации. Но теперь такой ветки нет.

Ещё одна подобная ошибка:

AWSSDK. Предупреждение PVS-Studio: V3022 Expression '!result' is always false. PCL SQLiteLocalStorage.cs 353 CognitoSync.

public void PutValue(....)
{ .... bool result = PutValueHelper(....); if (!result) <= { _logger.DebugFormat("{0}", @"Cognito Sync - SQLiteStorage - Put Value Failed"); } else { UpdateLastModifiedTimestamp(....); } ....
}

Анализатор утверждает, что значение переменной result всегда true. Такое возможно лишь в случае, если метод PutValueHelper будет всегда возвращать true. Посмотрим на этот метод:

private bool PutValueHelper(....)
{ .... if (....)) { return true; } if (record == null) { .... return true; } else { .... return true; }
}

Действительно, метод вернёт true при любом условии. Более того, анализатор выдал для этого метода предупреждение. Предупреждение PVS-Studio: V3009 [CWE-393] It's odd that this method always returns one and the same value of 'true'. SQLiteLocalStorage.cs 1016

Таким образом, анализатор был прав, указав на ошибку V3022 в вызывающем коде. Я умышленно не стал приводить это предупреждение ранее, когда рассматривал другие ошибки V3009, а приберёг его для этого случая.

Again Copy-Paste.

String' to the left and to the right of the '||' operator. Предупреждение PVS-Studio: V3001 There are identical sub-expressions 'this.token == JsonToken. Core. AWSSDK. Net45 JsonReader.cs 343

public bool Read()
{ .... if ( (this.token == JsonToken.ObjectEnd || this.token == JsonToken.ArrayEnd || this.token == JsonToken.String || // <= this.token == JsonToken.Boolean || this.token == JsonToken.Double || this.token == JsonToken.Int || this.token == JsonToken.UInt || this.token == JsonToken.Long || this.token == JsonToken.ULong || this.token == JsonToken.Null || this.token == JsonToken.String // <= )) { .... } ....
}

Дважды сравнивают поле this.token со значением JsonToken.String перечисления JsonToken. Вероятно, одно из сравнений должно содержать другое значение перечисления. Если это так, то допущена серьёзная ошибка.

Рефакторинг + невнимательность?

A different number of format items is expected while calling 'Format' function. Предупреждение PVS-Studio: V3025 [CWE-685] Incorrect format. AWSRegionKey. Arguments not used: AWSConfigs. Core. AWSSDK. Net45 AWSRegion.cs 116

public InstanceProfileAWSRegion()
{ .... if (region == null) { throw new InvalidOperationException( string.Format(CultureInfo.InvariantCulture, "EC2 instance metadata was not available or did not contain region information.", AWSConfigs.AWSRegionKey)); } ....
}

Вероятно, строка формата для метода string.Format ранее содержала спецификатор вывода {0}, для которого и был задан аргумент AWSConfigs.AWSRegionKey. Затем строку изменили, спецификатор пропал, а вот аргумент удалить забыли. Приведённый фрагмент кода работает без ошибок (исключение было бы выброшено в обратном случае — спецификатор без аргумента), но выглядит некрасиво. Код следует исправить следующим образом:

if (region == null)
{ throw new InvalidOperationException( "EC2 instance metadata was not available or did not contain region information.");
}

Unsafe

Consider assigning event to a local variable before invoking it. Предупреждение PVS-Studio: V3083 [CWE-367] Unsafe invocation of event 'mOnSyncSuccess', NullReferenceException is possible. CognitoSync. AWSSDK. PCL Dataset.cs 827

protected void FireSyncSuccessEvent(List<Record> records)
{ if (mOnSyncSuccess != null) { mOnSyncSuccess(this, new SyncSuccessEventArgs(records)); }
}

Достаточно распространённая ситуация небезопасного вызова обработчика события. Между проверкой переменной mOnSyncSuccess на равенство null и вызовом обработчика от события могут успеть отписаться, и его значение станет нулевым. Вероятность такого сценария мала, но код всё же лучше сделать более безопасным:

protected void FireSyncSuccessEvent(List<Record> records)
{ mOnSyncSuccess?.Invoke(this, new SyncSuccessEventArgs(records));
}

В коде есть и другие подобные ошибки.

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

  • V3083 [CWE-367] Unsafe invocation of event 'mOnSyncFailure', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.CognitoSync.PCL Dataset.cs 839
  • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.PCL AmazonServiceClient.cs 332
  • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.PCL AmazonServiceClient.cs 344
  • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.PCL AmazonServiceClient.cs 357
  • V3083 [CWE-367] Unsafe invocation of event 'mExceptionEvent', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.PCL AmazonServiceClient.cs 366
  • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.PCL AmazonWebServiceRequest.cs 78
  • V3083 [CWE-367] Unsafe invocation of event 'OnRead', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.PCL EventStream.cs 97
  • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.Android NetworkReachability.cs 57
  • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.Android NetworkReachability.cs 94
  • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.iOS NetworkReachability.cs 54

Недоработанный класс

AWSSDK. Предупреждение PVS-Studio: V3126 Type 'JsonData' implementing IEquatable<T> interface does not override 'GetHashCode' method. Net45 JsonData.cs 26 Core.

public class JsonData : IJsonWrapper, IEquatable<JsonData>
{ ....
}

Класс JsonData содержит достаточно много кода, поэтому я не стал приводить его целиком, ограничившись только объявлением. Этот класс действительно не содержит переопределённого метода GetHashCode, что небезопасно, так как может привести к ошибочному поведению при использовании типа JsonData для работы, например, с коллекциями. Вероятно, в настоящее время проблемы нет, но в дальнейшем стратегия использования этого типа может измениться. Более подробно данная ошибка описана в документации.

Заключение

NET при помощи статического анализатора PVS-Studio. Вот и все интересные ошибки, которые мне удалось обнаружить в коде AWS SDK для . Я обнаружил весьма небольшое число ошибок для 5 миллионов строк кода. Ещё раз подчеркну качество проекта. Но также весьма вероятно и то, что я зря причислил некоторые из описанных предупреждений к ошибкам. Хотя, вероятно, более тщательный анализ выданных предупреждений позволил бы мне добавить к этому списку ещё несколько ошибок. Однозначные выводы в таком случае всегда делает только разработчик, который находится в контексте проверяемого кода.

Searching for errors in the Amazon Web Services SDK source code for . Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Sergey Khrenov. NET

Теги
Показать больше

Похожие статьи

Добавить комментарий

Ваш e-mail не будет опубликован. Обязательные поля помечены *

Кнопка «Наверх»
Закрыть