# Searching for errors in the Amazon Web Services SDK source code for \.NET Welcome to all fans of trashing someone else's code\. :\) Today in our laboratory, we have a new material for a research \- the source code of the AWS SDK for \.NET project\. At the time, we wrote an article about checking AWS SDK for C\+\+\. Then there was not anything particularly interesting\. Let's see what \.NET of the AWS SDK version is worth\. Once again, it is a great opportunity to demonstrate the abilities of the PVS\-Studio analyzer and make the world a bit better\. ![0605_AWS_SDK_NET/image1.png](https://import.viva64.com/docx/blog/0605_AWS_SDK_NET/image1.png) Amazon Web Services \(AWS\) SDK for \.NET is a set of developer's tools, meant for creating applications based on \.NET in the AWS infrastructure\. This set enables to significantly simplify the process of code writing\. SDK includes sets API \.NET for various AWS services, such as Amazon S3, Amazon EC2, DynamoDB and others\. SDK source code is available on [GitHub](https://github.com/aws/aws-sdk-net)\. As I mentioned, at the time we have already written the [article](https://pvs-studio.com/en/blog/posts/0378/) about checking AWS SDK for C\+\+\. The article turned out to be small \- only a couple of errors found per 512 thousands of lines of code\. This time we are dealing with a much larger size of the code, which includes about 34 thousand of cs\-files, and the total number of lines of code \(excluding blank ones\) is impressive 5 million\. A small part of code \(200 thousand lines in 664\-cs files\) accrues to tests, I haven't considered them\. If the quality of the \.NET code of the SDK version is approximately the same as the one of C\+\+ \(two errors per 512 KLOC\), then we should get about 10 times greater number of errors\. Of course, this is a very inaccurate calculation methodology, which doesn't take into account the linguistic peculiarities and many other factors, but I don't think the reader now wants to go into boring reasoning\. Instead, I suggest moving on to the results\. The check was performed using PVS\-Studio 6\.27\. It is just incredible, but still the fact is that in the AWS SDK for \.NET the analyzer managed to detect 40 errors, which would be worth to talk about\. It demonstrates not only a high quality of the SDK code \(about 4 errors per 512 KLOC\), but also comparable quality of the C\# PVS\-Studio analyzer in comparison with C\+\+\. A great result\! Authors of AWS SDK for \.NET, you're real champs\! With each project, you demonstrate tremendous quality of the code\. It can be a great example for other teams\. However, of course, I would not be a developer of a static analyzer, if I didn't give my 2 cents\. :\) We are already working with a Lumberyard team from Amazon on the use of PVS\-Studio\. Since it is a very large company with a bunch of units around the world, it is very likely that the AWS SDK team for \.NET has never heard of PVS\-Studio\. Anyway, I have not found any signs of using our analyzer in the SDK code, although it doesn't say anything\. However, at least, the team [uses](https://aws.amazon.com/blogs/developer/code-analyzers-added-to-aws-sdk-for-net/?nc1=f_ls) the analyzer built into Visual Studio\. It is great, but code reviews can always be [enhanced](https://pvs-studio.com/en/blog/posts/csharp/0506/) :\)\. As a result, I did manage to find a few bugs in the SDK code and, finally, it's time to share them\. **Error in logic** **PVS\-Studio warning:** [V3008](https://pvs-studio.com/en/docs/warnings/v3008/) \[CWE\-563\] The 'this\.linker\.s3\.region' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 116, 114\. AWSSDK\.DynamoDBv2\.Net45 S3Link\.cs 116 ```cpp public string Region { get { .... } set { if (String.IsNullOrEmpty(value)) { this.linker.s3.region = "us-east-1"; } this.linker.s3.region = value; } } ``` The analyzer warns about repeated value assignment to the same variable\. From the code it becomes clear that this is due to the error that violates the logic of the program work: the value of the variable _this\.linker\.s3\.region_ will always be equal to the value of the variable _value_, regardless of the condition _if_ _\(String\.IsNullOrEmpty\(value\)\)_\. _return_ statement was missed in the body of _if_ block\. The code needs to be fixed as follows: ```cpp public string Region { get { .... } set { if (String.IsNullOrEmpty(value)) { this.linker.s3.region = "us-east-1"; return; } this.linker.s3.region = value; } } ``` **Infinite recursion** **PVS\-Studio warning:** [V3110](https://pvs-studio.com/en/docs/warnings/v3110/) \[CWE\-674\] Possible infinite recursion inside 'OnFailure' property\. AWSSDK\.ElasticMapReduce\.Net45 ResizeJobFlowStep\.cs 171 ```cpp OnFailure? onFailure = null; public OnFailure? OnFailure { get { return this.OnFailure; } // <= set { this.onFailure = value; } } ``` A classic example of a typo, which leads to an infinite recursion in the _get _accessor of the _OnFailure_ property\. Instead of returning the value of a private field _onFailure,_ the access to property _OnFailure_ takes place\. Correct variant: ```cpp public OnFailure? OnFailure { get { return this.onFailure; } set { this.onFailure = value; } } ``` You may ask: "How did it work?" So far \- no how\. The property is not used anywhere else, but this is temporary\. At one point, someone will start using it and will certainly receive an unexpected result\. To prevent such typos it is recommended not to use identifiers that differ only in case of the first letter\. Another comment to this construction is using of the identifier, which completely matches the name of the _OnFailure_ type\. From the point of view of the compiler, it is quite acceptable, but this complicates the perception of code for a person\. Another similar error: **PVS\-Studio warning:** [V3110](https://pvs-studio.com/en/docs/warnings/v3110/) \[CWE\-674\] Possible infinite recursion inside 'SSES3' property\. AWSSDK\.S3\.Net45 InventoryEncryption\.cs 37 ```cpp private SSES3 sSES3; public SSES3 SSES3 { get { return this.SSES3; } set { this.SSES3 = value; } } ``` The situation is identical to the described above\. However, here infinite recursion will occur when accessing the property _SSES3_ both for reading and assigning\. Correct variant: ```cpp public SSES3 SSES3 { get { return this.sSES3; } set { this.sSES3 = value; } } ``` **Test on consideration** Now I'd like to cite a task from a developer, taken with using Copy\-Paste method\. Take a look at the way code looks like in the Visual Studio editor, and try to find an error \(click on image to enlarge\)\. ![0605_AWS_SDK_NET/image3.png](https://import.viva64.com/docx/blog/0605_AWS_SDK_NET/image3.png) **PVS\-Studio warning:** [V3029](https://pvs-studio.com/en/docs/warnings/v3029/) The conditional expressions of the 'if' statements situated alongside each other are identical\. Check lines: 91, 95\. AWSSDK\.AppSync\.Net45 CreateApiKeyResponseUnmarshaller\.cs 91 I reduced the body of the method _UnmarshallException_, having removed everything that is not needed\. Now you can see that identical checks follow each other: ```cpp 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); } .... } ``` It might seem that the bug is not rough \- just an extra checking\. Nevertheless, often such a pattern may indicate more serious problems in the code, when a needed check will not be performed\. In the code, there are several similar errors\. **PVS\-Studio warnings:** * 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 **What are you?** **PVS\-Studio warning:** [V3062](https://pvs-studio.com/en/docs/warnings/v3062/) An object 'attributeName' is used as an argument to its own method\. Consider checking the first actual argument of the 'Contains' method\. AWSSDK\.MobileAnalytics\.Net45 CustomEvent\.cs 261 ```cpp /// /// Dictionary that stores attribute for this event only. /// private Dictionary _attributes = new Dictionary(); /// /// Gets the attribute. /// /// Attribute name. /// The attribute. Return null of attribute doesn't /// exist. 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; } ``` The analyzer has detected an error in the _GetAttribute_ method: a string is checked whether it contains itself\. From the description of the method it follows that if the attribute name \(_attributeName_ key\) is found \(in the dictionary _\_attributes_\), the attribute value should be returned, otherwise \- _null_\. In fact, as the condition _attributeName\.Contains\(attributeName\)_ is always true, an attempt is made to return the value by a key which might not be found in a dictionary\. Then, instead of returning _null,_ an exception _KeyNotFoundException_ will be thrown\. Let's try to fix this code\. To understand better how to do this, you should look at another method: ```cpp /// /// Determines whether this instance has attribute the specified /// attributeName. /// /// Attribute name. /// Return true if the event has the attribute, else /// false. public bool HasAttribute(string attributeName) { if(string.IsNullOrEmpty(attributeName)) { throw new ArgumentNullException("attributeName"); } bool ret = false; lock(_lock) { ret = _attributes.ContainsKey(attributeName); } return ret; } ``` This method checks whether the attribute name \(_attributeName_ key\) exists in the dictionary _\_attributes_\. Let's get back to the _GetAttribute_ method again and fix the error: ```cpp 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; } ``` Now the method does exactly what is stated in the description\. One more small comment to this fragment of code\. I noticed that the authors use _lock_ when working with the _\_attributes _dictionary\. It is clear that this is necessary when having a multithreaded access, but the _lock _construction is rather slow and cumbersome\. Instead of a _Dictionary_, in this case, perhaps, it would be more convenient to use thread\-safe version of the dictionary \- _ConcurrentDictionary_\. This way, there will be no need in _lock\._ Although, maybe I don't know about the specifics of the project\. **Suspicious behavior** **PVS\-Studio warning:** [V3063](https://pvs-studio.com/en/docs/warnings/v3063/) \[CWE\-571\] A part of conditional expression is always true if it is evaluated: string\.IsNullOrEmpty\(inferredIndexName\)\. AWSSDK\.DynamoDBv2\.PCL ContextInternal\.cs 802 ```cpp 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); .... } ``` The analyzer was concerned about the check _string\.IsNullOrEmpty\(inferredIndexName\)_\. Indeed, the string _inferredIndexName_ is assigned _null_, then the value of this variable isn't changed anywhere, then for some reason it is checked for _null _or an empty string\. Looks suspicious\. Let's take a close look at the above code fragment\. I deliberately did not reduce it to understand better the situation\. So, in the first _if _statement \(and also in the next one\) the variable _specifiedIndexName _is somehow checked\. Depending on the results of the checks, the variable _inferredIndexName_ is getting a new value\. Now let's look at the third _if_ statement\. The body of this statement \(throwing of the exception\) will be performed in case if _indexNames\.Count \> 0, _as the first part of the whole condition, which is _string\.IsNullOrEmpty\(inferredIndexName\)_ is always true\. Perhaps, variables _specifiedIndexName_ and _inferredIndexName _are mixed up or the third check has to be without _else_, representing a standalone _if_ statement: ```cpp 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(....); ``` In this case, it is difficult to give a definite answer on options to fix this code\. Anyway, the author needs to check it out\. **NullReferenceException** **PVS\-Studio warning:** [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) \[CWE\-476\] The 'conditionValues' object was used before it was verified against null\. Check lines: 228, 238\. AWSSDK\.Core\.Net45 JsonPolicyWriter\.cs 228 ```cpp private static void writeConditions(....) { .... foreach (....) { IList conditionValues = keyEntry.Value; if (conditionValues.Count == 0) // <= continue; .... if (conditionValues != null && conditionValues.Count != 0) { .... } .... } } ``` It's a classic\. The variable _conditionValues _is used without a preliminary check for _null_\. While later in the code this check is performed\. The code needs to be corrected as follows: ```cpp private static void writeConditions(....) { .... foreach (....) { IList conditionValues = keyEntry.Value; if (conditionValues != null && conditionValues.Count == 0) continue; .... if (conditionValues != null && conditionValues.Count != 0) { .... } .... } } ``` I found several similar errors in code\. **PVS\-Studio** **warnings:** * 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 The following warning is very similar in meaning, but the case is opposite to the one discussed above\. **PVS\-Studio warning:** [V3125](https://pvs-studio.com/en/docs/warnings/v3125/) \[CWE\-476\] The 'state' object was used after it was verified against null\. Check lines: 139, 127\. AWSSDK\.Core\.Net45 RefreshingAWSCredentials\.cs 139 ```cpp 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; // <= .... } ``` One of the code fragments includes checking the value of the _state _variable for _null_\. In the code below, the variable is used to unsubscribe from the _PreemptExpiryTime _event, however, a check for _null_ is no longer performed and throwing of the exception _NullReferenceException_ becomes possible\. A more secure version of the code: ```cpp 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; .... } ``` In the code, there are other similar errors: **PVS\-Studio warnings:** * 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 **Non\-alternate reality** **PVS\-Studio warning:** [V3009](https://pvs-studio.com/en/docs/warnings/v3009/) \[CWE\-393\] It's odd that this method always returns one and the same value of 'true'\. AWSSDK\.Core\.Net45 Lexer\.cs 651 ```cpp private static bool State19 (....) { while (....) { switch (....) { case '"': .... return true; case '\\': .... return true; default: .... continue; } } return true; } ``` The method always returns _true_\. Let's see how critical it is for the calling code\. I checked out the cases of using the_ State19 _method\. It is involved in filling the array of handlers _fsm\_handler\_table_ equally with other similar methods \(there are 28 of them with the names, respectively, starting from _State1_ to _State28_\)\. Here it is important to note that, in addition to _State19_, for some other handlers the warnings [V3009](https://pvs-studio.com/en/docs/warnings/v3009/) \[CWE\-393\] were issued as well\. These are handlers: _State23, State26, State27, State28_\. The warnings, issued by the analyzer for them: * 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 Here is the way the declaration and the array initialization of handlers look like: ```cpp private static StateHandler[] fsm_handler_table; .... private static void PopulateFsmTables () { fsm_handler_table = new StateHandler[28] { State1, State2, .... State19, .... State23, .... State26, State27, State28 }; ``` To complete the picture, let's see code of one of the handlers to which the analyzer haven't had any claims, for example, _State2_: ```cpp private static bool State2 (....) { .... if (....) { return true; } switch (....) { .... default: return false; } } ``` Here's the way how the call of handlers occurs: ```cpp public bool NextToken () { .... while (true) { handler = fsm_handler_table[state - 1]; if (! handler (fsm_context)) // <= throw new JsonException (input_char); .... } .... } ``` As we can see, an exception will be thrown in case of returning _false_\. In our case, for the handlers _State19, State23, State26 State27_ and_ State28 _this will never happen\. Looks suspicious\. On the other hand, five handlers have similar behavior \(will always return _true_\), so maybe it was so contrived and is not the result of a typo\. Why am I going so deep in all this? This situation is very significant in the sense that the static analyzer often can only indicate a suspicious construction\. And even a person \(not a machine\), who does not have sufficient knowledge about the project, is still not able to give a full answer on the presence of the error, even having spent time learning code\. A developer should review this code\. **Meaningless checks** **PVS\-Studio warning:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) \[CWE\-571\] Expression 'doLog' is always true\. AWSSDK\.Core\.Net45 StoredProfileAWSCredentials\.cs 235 ```cpp private static bool ValidCredentialsExistInSharedFile(....) { .... var doLog = false; try { if (....) { return true; } else { doLog = true; } } catch (InvalidDataException) { doLog = true; } if (doLog) // <= { .... } .... } ``` Pay attention to the _doLog _variable\. After initialization with the _false_ value, this variable will get the _true _value in all cases further along the code\. Therefore, the check _if \(doLog\)_ is always true\. Perhaps, earlier in the method there was a branch, in which the _doLog _variable wasn't assigned any value\. At the moment of checking it could contain the _false_ value, received when initializing\. But now there is no such a branch\. Another similar error: **PVS\-Studio warning:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression '\!result' is always false\. AWSSDK\.CognitoSync\.PCL SQLiteLocalStorage\.cs 353 ```cpp public void PutValue(....) { .... bool result = PutValueHelper(....); if (!result) <= { _logger.DebugFormat("{0}", @"Cognito Sync - SQLiteStorage - Put Value Failed"); } else { UpdateLastModifiedTimestamp(....); } .... } ``` The analyzer claims that the value of the _result _variable is always true\. It is possible only in case if the method _PutValueHelper_ will always return _true_\. Take a look at this method: ```cpp private bool PutValueHelper(....) { .... if (....)) { return true; } if (record == null) { .... return true; } else { .... return true; } } ``` Indeed, the method will return _true_ under all conditions\. Moreover, the analyzer has issued a warning for this method\. **PVS\-Studio warning:** [V3009](https://pvs-studio.com/en/docs/warnings/v3009/) \[CWE\-393\] It's odd that this method always returns one and the same value of 'true'\. SQLiteLocalStorage\.cs 1016 I deliberately did not cite this warning earlier when I was inquiring into other bugs [V3009](https://pvs-studio.com/en/docs/warnings/v3009/) and saved it up for this case\. Thus, the tool was right to point out the error [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) in the calling code\. **Copy\-Paste\. Again** **PVS\-Studio warning:** [V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'this\.token \=\= JsonToken\.String' to the left and to the right of the '\|\|' operator\. AWSSDK\.Core\.Net45 JsonReader\.cs 343 ```cpp 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 // <= )) { .... } .... } ``` The field _this\.token _is compared twice with the value _JsonToken\.String _of the enumeration _JsonToken_\. Probably, one of the comparisons should contain another enumeration value\. If so, a serious mistake has been made here\. **Refactoring \+ inattention?** **PVS\-Studio warning:** [V3025](https://pvs-studio.com/en/docs/warnings/v3025/) \[CWE\-685\] Incorrect format\. A different number of format items is expected while calling 'Format' function\. Arguments not used: AWSConfigs\.AWSRegionKey\. AWSSDK\.Core\.Net45 AWSRegion\.cs 116 ```cpp 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)); } .... } ``` Perhaps, the format string for the _string\.Format _method previously contained the format item _\{0\}, _for which the argument _AWSConfigs\.AWSRegionKey_ was set\. Then the string was changed, the format item was gone, but a developer forgot to remove the argument\. The given code example works without errors \(the exception was thrown in the opposite case \- the format item without the argument\), but it looks not nice\. The code should be corrected as follows: ```cpp if (region == null) { throw new InvalidOperationException( "EC2 instance metadata was not available or did not contain region information."); } ``` **Unsafe** **PVS\-Studio warning:** [V3083](https://pvs-studio.com/en/docs/warnings/v3083/) \[CWE\-367\] Unsafe invocation of event 'mOnSyncSuccess', NullReferenceException is possible\. Consider assigning event to a local variable before invoking it\. AWSSDK\.CognitoSync\.PCL Dataset\.cs 827 ```cpp protected void FireSyncSuccessEvent(List records) { if (mOnSyncSuccess != null) { mOnSyncSuccess(this, new SyncSuccessEventArgs(records)); } } ``` A common situation of an unsafe call of the event handler\. A user is able to unsubscribe between the checking of the variable _mOnSyncSuccess _for _null_ and calling of a handler, so its value will become _null_\. The likelihood of such a scenario is small, but it is still better to make code more secure: ```cpp protected void FireSyncSuccessEvent(List records) { mOnSyncSuccess?.Invoke(this, new SyncSuccessEventArgs(records)); } ``` In the code, there are other similar errors: **PVS\-Studio warnings:** * 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 **Crude class** **PVS\-Studio warning:** [V3126](https://pvs-studio.com/en/docs/warnings/v3126/) Type 'JsonData' implementing IEquatable interface does not override 'GetHashCode' method\. AWSSDK\.Core\.Net45 JsonData\.cs 26 ```cpp public class JsonData : IJsonWrapper, IEquatable { .... } ``` The _JsonData _class contains quite a lot of code, so I didn't give it in whole, citing just its declaration\. This class really does not contain the overridden method _GetHashCode, _which is unsafe, as it can lead to erroneous behavior when using the _JsonData_ type for working, for example, with collections\. Probably, there is no problem at the moment, but in future this type of strategy might change\. This error is described in the [documentation](https://pvs-studio.com/en/docs/warnings/v3126/) in more detail\. **Conclusion** These are all interesting bugs that I was able to detect in the code of AWS SDK for \.NET using the PVS\-Studio static analyzer\. I'd like to highlight once again the quality of the project\. I found a very small number of errors for 5 million lines of code\. Although probably more thorough analysis of issued warnings would let me add a few more errors to this list\. Nevertheless, it is also quite likely that I added on some of the warnings to errors for nothing\. Unambiguous conclusions in this case are always made only by a developer who is in the context of the checked code\.