Azure PowerShell:per lo più innocuo

Azure PowerShell:per lo più innocuo

Ciao a tutti. Oggi abbiamo un altro progetto Microsoft all'asta. Dal titolo di questo articolo, puoi intuire che questa volta gli sviluppatori non ci hanno "soddisfatto" con un gran numero di errori. Ci auguriamo che gli autori del progetto non si offendano per il titolo. Dopotutto, un piccolo numero di errori è ottimo, vero? Tuttavia, siamo comunque riusciti a trovare qualcosa di interessante nel codice di Azure PowerShell. Suggeriamo di conoscere le caratteristiche di questo progetto e di controllare gli errori rilevati utilizzando l'analizzatore C# PVS-Studio.

Informazioni sul progetto

Azure PowerShell è un set di comandi (cmdlet) che consente di gestire le risorse di Azure direttamente dalla riga di comando di PowerShell. Lo scopo principale di questo set è semplificare il processo di studio e avviare rapidamente lo sviluppo per Azure. Azure PowerShell offre ad amministratori e sviluppatori funzionalità interessanti per creare, distribuire e gestire applicazioni Microsoft Azure.

Azure PowerShell è sviluppato in ambiente .NET Standard, è supportato da PowerShell 5.1 per Windows e PowerShell 6.xe versioni successive per tutte le piattaforme. Il codice sorgente di Azure PowerShell è disponibile su GitHub.

Recentemente ho ricevuto spesso progetti Microsoft per un controllo. A mio parere, la qualità di questi progetti è solitamente di prim'ordine. Anche se, ovviamente, non senza eccezioni, come descritto nell'articolo "WinForms:errori, Holmes". Ma questa volta va tutto bene. Il progetto è grande:6845 file di codice sorgente .cs contengono circa 700.000 righe, escluse quelle vuote (non ho tenuto conto di test e avvisi di terzo livello di certezza). Sono stati trovati pochissimi errori per una tale quantità di codice:non più di cento. C'erano molti casi simili, quindi ho scelto quelli più interessanti per l'articolo. Come al solito, ho ordinato gli errori in base ai numeri degli avvisi di PVS-Studio.

Inoltre mi sono imbattuto in alcuni frammenti di codice che sembravano errori, ma non potevano essere riconosciuti come decisamente errati, poiché non ho abbastanza familiarità con le peculiarità dello sviluppo di PowerShell. Spero che tra i lettori ci saranno specialisti in questo numero che mi aiuteranno. Lo descriverò in dettaglio di seguito.

Prima della parte di feedback, vorrei citare la struttura specifica del progetto. Il codice sorgente di Azure PowerShell è costituito da più di settanta soluzioni di Visual Studio. Alcune soluzioni includono progetti di altre. Questa struttura ha rallentato un po' (non molto) l'analisi. Tuttavia il controllo non ha causato altre difficoltà. Per comodità, nel messaggio di errore (tra parentesi) indicherò il nome della soluzione dove è stato riscontrato l'errore.

Risultati dell'analisi

V3001 Esistono sottoespressioni identiche 'strTimespan.Contains("M")' a sinistra ea destra di '||' operatore. AzureServiceBusCmdletBase.cs 187 (EventGrid)

public static TimeSpan ParseTimespan(string strTimespan)
{
  ....
  if (strTimespan.Contains("P") 
    || strTimespan.Contains("D") 
    || strTimespan.Contains("T") 
    || strTimespan.Contains("H") 
    || strTimespan.Contains("M") 
    || strTimespan.Contains("M"))
  ....
}

Un esempio di un errore abbastanza ovvio che solo uno sviluppatore può correggere. In questo caso, non è assolutamente chiaro se abbiamo a che fare con la duplicazione del codice che non ha alcun effetto o se deve avvenire qualcos'altro invece di "M" in uno dei due ultimi controlli.

V3001 Sono presenti sottoespressioni identiche 'this.AggregationType !=null' a sinistra ea destra dell'operatore '&&'. GetAzureRmMetricCommand.cs 156 (Monitoraggio)

public AggregationType? AggregationType { get; set; }
....
protected override void ProcessRecordInternal()
{
  ....
  string aggregation = (this.AggregationType != null &&
    this.AggregationType.HasValue) ?
    this.AggregationType.Value.ToString() : null;
  ....
}

Probabilmente non ci sono errori qui. Questo è un esempio di codice ridondante. A volte tale codice potrebbe indicare la mancanza di conoscenza dello sviluppatore. Il punto è che i controlli this.AggregationType !=null e this.AggregationType.HasValue sono identici. È sufficiente usarne solo uno (qualsiasi). Personalmente, preferisco l'opzione con HasValue:

string aggregation = this.AggregationType.HasValue ?
  this.AggregationType.Value.ToString() : 
  null;

V3003 È stato rilevato l'utilizzo del pattern 'if (A) {...} else if (A) {...}'. C'è una probabilità di presenza di un errore logico. Righe di controllo:152, 163. GetAzureRmRecoveryServicesBackupProtectionPolicy.cs 152 (RecoveryServices)

public override void ExecuteCmdlet()
{
  ....
  if( WorkloadType == Models.WorkloadType.AzureVM )
  {
    ....
  }
  ....
  else if( WorkloadType == Models.WorkloadType.AzureFiles )
  {
    if( BackupManagementType != Models.BackupManagementType.AzureStorage )
    {
      throw new ArgumentException(
        Resources.AzureFileUnsupportedBackupManagementTypeException );
    }
    serviceClientProviderType = ServiceClientHelpers.
      GetServiceClientProviderType( Models.WorkloadType.AzureFiles );
  }
  else if( WorkloadType == Models.WorkloadType.AzureFiles )
  {
    if( BackupManagementType != Models.BackupManagementType.AzureStorage )
    {
      throw new ArgumentException(
        Resources.AzureFileUnsupportedBackupManagementTypeException );
    }
    serviceClientProviderType = ServiceClientHelpers.
      GetServiceClientProviderType( Models.WorkloadType.AzureFiles );
  }
  ....
}

Due altri se i blocchi sono assolutamente identici, includendo sia la condizione che il corpo del blocco. Tali errori vengono generalmente commessi quando si utilizza il metodo copia-incolla. Il problema qui è ancora una volta la criticità dell'errore. Se non si tratta di una semplice duplicazione del codice, potrebbe essere il controllo necessario assente e l'appropriato insieme di azioni. L'autore deve assolutamente modificare il codice.

V3005 La variabile 'this.VM.OSProfile.WindowsConfiguration.ProvisionVMAgent' viene assegnata a se stessa. SetAzureVMOperatingSystemCommand.cs 298 (calcolo)

public override void ExecuteCmdlet()
{
  ....
  // OS Profile
  this.VM.OSProfile.WindowsConfiguration.ProvisionVMAgent =
    this.VM.OSProfile.WindowsConfiguration.ProvisionVMAgent;
  ....
}

Il valore dell'immobile è autoassegnato. Dai un'occhiata alla sua dichiarazione:

[JsonProperty(PropertyName = "provisionVMAgent")]
public bool? ProvisionVMAgent { get; set; }

La Proprietà Json la descrizione afferma:"Istruisce a Newtonsoft.Json.JsonSerializer di serializzare sempre il membro con il nome specificato". Sembra che tutto sia innocente e che sia stato commesso l'errore evidente. Utilizzo esplicito di questo anche l'accesso alla proprietà è abbastanza confuso. Forse, un'altra variabile non è stata erroneamente specificata al posto di this. Ma non saltiamo alle conclusioni. Il fatto è che mi sono imbattuto in molti di questi incarichi (una proprietà è auto-assegnata). Ecco un esempio di assegnazione, molto simile a un errore:

V3005 La variabile 'this.LastHeartbeat' è assegnata a se stessa. PSFabricDetails.cs 804 (RecoveryServices)

public ASRInMageAzureV2SpecificRPIDetails(
  InMageAzureV2ReplicationDetails details)
{
  this.LastHeartbeat = this.LastHeartbeat;  // <=
  this.RecoveryAvailabilitySetId = details.RecoveryAvailabilitySetId;
  this.AgentVersion = details.AgentVersion;
  this.DiscoveryType = details.DiscoveryType;
  ....
}

Diamo un'occhiata più da vicino al secondo e ai successivi incarichi. Nella parte destra dell'espressione, dettagli avviene invece di questo. Ora guarda la dichiarazione di this.LastHeartbeat proprietà:

public DateTime? LastHeartbeat { get; set; }

Infine, troviamo la proprietà con lo stesso nome in InMageAzureV2ReplicationDetails classe. Tale proprietà è dichiarata lì:

public class InMageAzureV2ReplicationDetails :
  ReplicationProviderSpecificSettings
{
  ....
  [JsonProperty(PropertyName = "lastHeartbeat")]
  public DateTime? LastHeartbeat { get; set; }
  ....  
}

Bene, in questo caso, sono disposto ad ammettere che è un vero errore. Ma cosa dobbiamo fare con i prossimi avvisi? A differenza di due frammenti di codice precedenti, esistono più proprietà autoassegnate. Bene, questo sembra meno un errore:

  • V3005 La variabile 'this.ResourceGroupName' è assegnata a se stessa. RemoveAzureRmExpressRouteConnectionCommand.cs 84 (CognitiveServices)
  • V3005 La variabile 'this.ExpressRouteGatewayName' è assegnata a se stessa. RemoveAzureRmExpressRouteConnectionCommand.cs 85 (CognitiveServices)
  • V3005 La variabile 'this.Name' è assegnata a se stessa. RemoveAzureRmExpressRouteConnectionCommand.cs 86 (CognitiveServices)
[Cmdlet(VerbsCommon.Remove,
  ResourceManager.Common.AzureRMConstants.AzureRMPrefix +
    "ExpressRouteConnection",
  DefaultParameterSetName =
    CortexParameterSetNames.ByExpressRouteConnectionName,
  SupportsShouldProcess = true),
  OutputType(typeof(bool))]
public class RemoveExpressRouteConnectionCommand :
  ExpressRouteConnectionBaseCmdlet
{
  [Parameter(
    Mandatory = true,
    ParameterSetName = CortexParameterSetNames.ByExpressRouteConnectionName,
    HelpMessage = "The resource group name.")]
  [ResourceGroupCompleter]
  [ValidateNotNullOrEmpty]
  public string ResourceGroupName { get; set; }
  ....
  public override void Execute()
  {
    if (....)
    {
      this.ResourceGroupName = this.ResourceGroupName;
      this.ExpressRouteGatewayName = this.ExpressRouteGatewayName;
      this.Name = this.Name;
    }
    ....
  }    
  ....
}

Il Esegui contiene tre autoassegnazioni di proprietà di seguito. Per ogni evenienza, ho citato la dichiarazione completa della classe RemoveExpressRouteConnectionCommand e tutti i suoi attributi, nonché ResourceGroupName dichiarazione di proprietà (le altre due proprietà sono dichiarate in modo simile). Sono stati questi avvertimenti che mi hanno fatto riflettere sulla domanda:"È un errore?" Sospetto che qui si stia verificando un po' di magia interna dello sviluppo di PowerShell. Spero che tra i lettori ci saranno esperti che sono informati su questo problema. Non sono pronto a trarre conclusioni in questo caso.

V3006 L'oggetto è stato creato ma non viene utilizzato. Potrebbe mancare la parola chiave 'throw':throw new ArgumentException(FOO). StartAzureRmRecoveryServicesAsrTestFailoverJob.cs 259 (RecoveryServices)

private void StartRPITestFailover()
{
  ....
  if (....)
  {
    ....
  }
  else
  {
    new ArgumentException(
      Resources
        .UnsupportedDirectionForTFO); // Throw Unsupported Direction
                                      // Exception
  }
  ....
}

Il lancio la parola chiave è omessa. E il commento dice che l'eccezione deve solo essere generata. Ho riscontrato molti altri errori simili in RecoveryServices soluzione:

  • V3006 L'oggetto è stato creato ma non viene utilizzato. Potrebbe mancare la parola chiave 'throw':throw new ArgumentException(FOO). StartAzureRmRecoveryServicesAsrTestFailoverJob.cs 305 (RecoveryServices)
  • V3006 L'oggetto è stato creato ma non viene utilizzato. Potrebbe mancare la parola chiave 'throw':throw new ArgumentException(FOO). StartAzureRmRecoveryServicesAsrUnPlannedFailover.cs 278 (RecoveryServices)
  • V3006 L'oggetto è stato creato ma non viene utilizzato. Potrebbe mancare la parola chiave 'throw':throw new ArgumentException(FOO). StartAzureRmRecoveryServicesAsrUnPlannedFailover.cs 322 (RecoveryServices)
  • V3006 L'oggetto è stato creato ma non viene utilizzato. Potrebbe mancare la parola chiave 'throw':throw new ArgumentException(FOO). UpdateAzureRmRecoveryServicesAsrProtectionDirection.cs 421 (RecoveryServices)
  • V3006 L'oggetto è stato creato ma non viene utilizzato. Potrebbe mancare la parola chiave 'throw':throw new ArgumentException(FOO). UpdateAzureRmRecoveryServicesAsrProtectionDirection.cs 452 (RecoveryServices)

V3022 L'espressione 'apiType.HasValue' è sempre falsa. ApiManagementClient.cs 1134 (ApiManagement)

private string GetApiTypeForImport(...., PsApiManagementApiType? apiType)
{
  ....
  if (apiType.HasValue)
  {
    switch(apiType.Value)
    {
      case PsApiManagementApiType.Http: return SoapApiType.SoapToRest;
      case PsApiManagementApiType.Soap: return SoapApiType.SoapPassThrough;
      default: return SoapApiType.SoapPassThrough;
    }
  }

  return apiType.HasValue ?        // <=
    apiType.Value.ToString("g") : 
    PsApiManagementApiType.Http.ToString("g");
}

La logica del lavoro è stata infranta. Se apiType contiene un valore, il controllo non raggiunge il ritorno espressione alla fine del metodo (tutti switch le filiali contengono rendimento ). In caso contrario, il metodo restituirà sempre PsApiManagementApiType.Http.ToString("g") , mentre apiType.Value.ToString("g") il valore non verrà mai restituito.

V3022 L'espressione 'automationJob !=null &&automationJob ==null' è sempre falsa. NodeConfigurationDeployment.cs 199 (Automazione)

public NodeConfigurationDeployment(
  ....,
  Management.Automation.Models.Job automationJob = null, 
  ....)
{
  ....
  if (automationJob != null && automationJob == null) return;
  ....
}

Codice controintuitivo. Due controlli che si contraddicono. Probabilmente il secondo controllo per null contiene la variabile sbagliata.

L'espressione V3022 è sempre falsa. DataFactoryClient.Encrypt.cs 37 (DataFactory)

public virtual string OnPremisesEncryptString(....)
{
  ....
  if ( linkedServiceType == LinkedServiceType.OnPremisesSqlLinkedService 
    && linkedServiceType == LinkedServiceType.OnPremisesOracleLinkedService
    && linkedServiceType == LinkedServiceType.OnPremisesFileSystemLinkedService
    && (value == null || value.Length == 0))
  {
    throw new ArgumentNullException("value");
  }
  ....
}

Il controllo è inutile e l'eccezione non verrà mai generata. La condizione richiede linkedServiceType simultanea uguaglianza della variabile a tre diversi valori. Gli operatori &&e || rischiano di essere confusi. Codice fisso:

if (( linkedServiceType == LinkedServiceType.OnPremisesSqlLinkedService 
  || linkedServiceType == LinkedServiceType.OnPremisesOracleLinkedService
  || linkedServiceType == LinkedServiceType.OnPremisesFileSystemLinkedService)
  && (value == null || value.Length == 0))
....

V3022 L'espressione 'Ekus ==null' è sempre falsa. PSKeyVaultCertificatePolicy.cs 129 (KeyVault)

internal CertificatePolicy ToCertificatePolicy()
{
  ....
  if (Ekus != null)
  {
    x509CertificateProperties.Ekus = Ekus == null ? 
      null : new List<string>(Ekus);
  }                
  ....
}

Controllo ridondante di Ekus variabile per null . Probabilmente va bene, ma il codice non ha un bell'aspetto.

V3023 Considerare di esaminare questa espressione. L'espressione è eccessiva o contiene un errore di stampa. PolicyRetentionObjects.cs 207 (RecoveryServices)

public virtual void Validate()
{
  if (RetentionTimes == null 
    || RetentionTimes.Count == 0 
    || RetentionTimes.Count != 1)
  {
    throw new ArgumentException(
      Resources.InvalidRetentionTimesInPolicyException);
  }
}

Ecco un controllo eccessivo o una condizione eccessiva. Il controllo RetentionTimes.Count ==0 è inutile, poiché dopo di ciò, il controllo RetentionTimes.Count !=1 segue.

V3025 Formato errato. È previsto un numero diverso di elementi di formato durante la chiamata alla funzione 'Formatta'. Argomenti non utilizzati:this.ResourceGroupName. NewScheduledQueryRuleCommand.cs 117 (Monitoraggio)

protected override void ProcessRecordInternal()
{
  ....
  if (this.ShouldProcess(this.Name,
    string.Format("Creating Log Alert Rule '{0}' in resource group {0}",
      this.Name, this.ResourceGroupName)))
  {
    ....
  }
  ....
}

Un errore nella riga di formattazione. Lo specificatore {0} viene utilizzato due volte e il Formato al metodo vengono passati due argomenti. Ecco la versione corretta:

if (this.ShouldProcess(this.Name,
  string.Format("Creating Log Alert Rule '{0}' in resource group {1}",
    this.Name, this.ResourceGroupName)))
....

Un altro errore simile:

  • V3025 Formato errato. È previsto un numero diverso di elementi di formato durante la chiamata alla funzione 'Formatta'. Argomenti non utilizzati:this.ResourceGroupName. RemoveScheduledQueryRuleCommand.cs 88 (Monitoraggio)

V3042 Possibile NullReferenceException. Il '?.' e '.' gli operatori vengono utilizzati per accedere ai membri dell'oggetto 'imageAndOsType' VirtualMachineScaleSetStrategy.cs 81 (Compute)

internal static ResourceConfig<VirtualMachineScaleSet> 
CreateVirtualMachineScaleSetConfig(...., ImageAndOsType imageAndOsType, ....)
{
  ....
  VirtualMachineProfile = new VirtualMachineScaleSetVMProfile
  {
    OsProfile = new VirtualMachineScaleSetOSProfile
    {
        ....,
        WindowsConfiguration = 
          imageAndOsType.CreateWindowsConfiguration(),  // <=
        ....,
    },
    StorageProfile = new VirtualMachineScaleSetStorageProfile
    {
        ImageReference = imageAndOsType?.Image,  // <=
        DataDisks = DataDiskStrategy.CreateVmssDataDisks(
          imageAndOsType?.DataDiskLuns, dataDisks)  // <=
    },  
  },
  ....
}

Quando si crea il VirtualMachineScaleSetVMProfile oggetto, il imageAndOsType la variabile è controllata per null senza alcun controllo preliminare. Tuttavia, ulteriormente durante la creazione di VirtualMachineScaleSetStorageProfile , questa variabile è già verificata utilizzando l'operatore di accesso condizionale anche due volte. Il codice non sembra sicuro.

V3042 Possibile NullReferenceException. Il '?.' e '.' gli operatori vengono utilizzati per accedere ai membri dell'oggetto 'existingContacts' RemoveAzureKeyVaultCertificateContact.cs 123 (KeyVault)

public override void ExecuteCmdlet()
{
  ....
  List<PSKeyVaultCertificateContact> existingContacts;
  
  try
  {
    existingContacts = this.DataServiceClient.
                       GetCertificateContacts(VaultName)?.ToList();
  }
  catch (KeyVaultErrorException exception)
  {
    ....
    existingContacts = null;
  }
  
  foreach (var email in EmailAddress)
  {
    existingContacts.RemoveAll(....);  // <=
  }
  ....
}

Sia nell'esecuzione normale che come risultato della gestione di un'eccezione, la variabile existingContacts può ottenere il null valore, dopo di che l'esecuzione continuerà. Più avanti nel codice, questa variabile viene utilizzata senza alcun motivo specifico.

V3066 Possibile ordine errato degli argomenti passati al metodo 'PersistSyncServerRegistration':'storageSyncServiceUid' e 'discoveryUri'. EcsManagementInteropClient.cs 364 (StorageSync)

public class EcsManagementInteropClient : IEcsManagement
{
  ....
  public int PersistSyncServerRegistration(....)
  {
    return m_managementObject.PersistSyncServerRegistration(
      serviceUri,
      subscriptionId,
      storageSyncServiceName,
      resourceGroupName,
      clusterId,
      clusterName,
      storageSyncServiceUid,  // <=
      discoveryUri,           // <=
      serviceLocation,
      resourceLocation);
  }
  ....
}

L'analizzatore sospettava che l'ordine degli argomenti di PersistSyncServerRegistration il metodo è confuso. Dichiarazione del metodo:

public interface IEcsManagement : IDisposable
{
  ....
  int PersistSyncServerRegistration(
    [In, MarshalAs(UnmanagedType.BStr)]
    string serviceUri,
    [In, MarshalAs(UnmanagedType.BStr)]
    string subscriptionId,
    [In, MarshalAs(UnmanagedType.BStr)]
    string storageSyncServiceName,
    [In, MarshalAs(UnmanagedType.BStr)]
    string resourceGroupName,
    [In, MarshalAs(UnmanagedType.BStr)]
    string clusterId,
    [In, MarshalAs(UnmanagedType.BStr)]
    string clusterName,
    [In, MarshalAs(UnmanagedType.BStr)]
    string discoveryUri,                              // <=
    [In, MarshalAs(UnmanagedType.BStr)]
    string storageSyncServiceUid,                     // <=
    [In, MarshalAs(UnmanagedType.BStr)]
    string serviceLocation,
    [In, MarshalAs(UnmanagedType.BStr)]
    string resourceLocation);
  ....
}

In effetti, qui c'è qualcosa che non va con gli argomenti numero sette e otto. L'autore deve controllare il codice.

V3077 Il setter della proprietà 'GetGuid' non utilizza il suo parametro 'value'. RecoveryServicesBackupCmdletBase.cs 54 (RecoveryServices)

public abstract class RecoveryServicesBackupCmdletBase : AzureRMCmdlet
{
  ....
  static string _guid;
  
  protected static string GetGuid
  {
    get { return _guid; }
    set { _guid = Guid.NewGuid().ToString(); }
  }
  ....
}

Il setter non usa il parametro passato. Invece, crea un nuovo GUID e lo assegna a _guid campo. Penso che la maggior parte dei lettori sarebbe d'accordo sul fatto che tale codice sembri almeno brutto. Questa costruzione non è molto comoda da usare:quando si (ri)inizializza GetGuid proprietà, bisogna assegnargli un valore falso, che non è molto evidente. Ma soprattutto mi ha divertito il modo in cui gli autori hanno utilizzato questo schema. C'è solo un posto, dove GetGuid viene gestito. Dai un'occhiata:

public override void ExecuteCmdlet()
{
  ....
  var itemResponse = ServiceClientAdapter.CreateOrUpdateProtectionIntent(
    GetGuid ?? Guid.NewGuid().ToString(),
    ....);  
  ....
}

Geniale!

V3091 Analisi empirica. È possibile che sia presente un errore di battitura all'interno della stringa letterale:"Management Group Id". La parola "Id" è sospetta. Constants.cs 36 (Risorse)

public class HelpMessages
{
  public const string SubscriptionId = "Subscription Id of the subscription
                                        associated with the management";
  public const string GroupId = "Management Group Id";  // <=
  public const string Recurse = "Recursively list the children of the
                                 management group";
  public const string ParentId = "Parent Id of the management group";
  public const string GroupName = "Management Group Id";  // <=
  public const string DisplayName = "Display Name of the management group";
  public const string Expand = "Expand the output to list the children of the
                                management group";
  public const string Force = "Force the action and skip confirmations";
  public const string InputObject = "Input Object from the Get call";
  public const string ParentObject = "Parent Object";
}

L'analizzatore ha indicato un possibile errore nella stringa assegnata per GroupName costante. La conclusione si basa sull'analisi empirica di altri incarichi, tenendo conto dei nomi delle variabili. Penso che in questo caso l'analizzatore abbia ragione e il valore di GroupName costante dovrebbe essere una sorta di "nome del gruppo di gestione". Probabilmente l'errore è dovuto al fatto che il valore per GroupId costante è stata copiata, ma non modificata.

Un altro errore simile:

  • V3091 Analisi empirica. È possibile che sia presente un errore di battitura all'interno della stringa letterale. La parola "Nome" è sospetta. ParamHelpMsgs.cs 153 (Servizi di ripristino)

V3093 Il '|' l'operatore valuta entrambi gli operandi. Forse un cortocircuito '||' dovrebbe essere utilizzato invece l'operatore. PSKeyVaultCertificatePolicy.cs 114 (KeyVault)

internal CertificatePolicy ToCertificatePolicy()
{
  ....
  if (!string.IsNullOrWhiteSpace(SubjectName) ||
    DnsNames != null ||
    Ekus != null ||
    KeyUsage != null |        // <=
    ValidityInMonths.HasValue)
  {
    ....
  }
  ....
}

In questo frammento potrebbe verificarsi un errore e in if blocco tra due ultime condizioni il || potrebbe essere stato utilizzato l'operatore. Ma come spesso accade, solo lo sviluppatore può dare la risposta giusta.

V3095 L'oggetto 'certificato' è stato utilizzato prima di essere verificato rispetto a null. Righe di controllo:41, 43. CertificateInfo.cs 41 (Automazione)

public CertificateInfo(
  ...., 
  Azure.Management.Automation.Models.Certificate certificate)
{
  ....
  this.Name = certificate.Name;
  
  if (certificate == null) return;
  ....
}

Classico. Per prima cosa viene utilizzato l'oggetto e solo dopo viene verificato che il riferimento sia null . Ci imbattiamo molto spesso in tali errori. Consideriamo un altro errore simile.

V3095 L'oggetto 'clusterCred' è stato utilizzato prima di essere verificato rispetto a null. Righe di controllo:115, 118. InvokeHiveCommand.cs 115 (HDInsight)

public override void ExecuteCmdlet()
{
  ....
  _credential = new BasicAuthenticationCloudCredentials
  {
    Username = clusterCred.UserName,
    Password = clusterCred.Password.ConvertToString()
  };
  
  if (clusterConnection == null || clusterCred == null)
  ....
}

Ecco un paio di errori simili:

  • V3095 L'oggetto '_profile' è stato utilizzato prima di essere verificato rispetto a null. Righe di controllo:47, 49. RMProfileClient.cs 47 (Account)
  • V3095 L'oggetto 'this.LoadBalancer.BackendAddressPools' è stato utilizzato prima di essere verificato rispetto a null. Righe di controllo:56, 63. AddAzureRmLoadBalancerBackendAddressPoolConfigCommand.cs 56 (CognitiveServices)
  • In generale, ho riscontrato molti errori V3095 nel codice di Azure PowerShell. Ma sono tutti abbastanza simili, quindi non mi soffermerò su questo problema.

V3125 L'oggetto 'startTime' è stato utilizzato dopo che è stato verificato rispetto a null. Linee di controllo:1752, 1738. AutomationPSClientDSC.cs 1752 (Automazione)

private string GetNodeReportListFilterString(
  ....,
  DateTimeOffset? startTime,
  ....,
  DateTimeOffset? lastModifiedTime)
{
  ....
  if (startTime.HasValue)
  {
    odataFilter.Add("properties/startTime ge " +
      this.FormatDateTime(startTime.Value));      // <=
  }
  ....
  if (lastModifiedTime.HasValue)
  {
    odataFilter.Add("properties/lastModifiedTime ge " +
      this.FormatDateTime(startTime.Value));      // <=
  }
  ....
}

È anche un tipo di errore piuttosto diffuso. L'ora di inizio la variabile viene verificata per la presenza di un valore quando viene utilizzata per la prima volta. Ma non è fatto nell'uso successivo. Bene, la situazione può essere anche peggiore. Guarda il secondo se bloccare. Penso che startTime la variabile non deve essere affatto qui. In primo luogo, non viene verificata la presenza di un valore prima del suo utilizzo. In secondo luogo, la stringa formata per essere passata a Add il metodo conferma anche il mio suggerimento. Un'altra variabile (lastModifiedTime ) è menzionato nella prima parte di questa stringa.

V3125 L'oggetto 'firstPage' è stato utilizzato dopo che è stato verificato rispetto a null. Linee di controllo:113, 108. IntegrationAccountAgreementOperations.cs 113 (LogicApp)

public IList<IntegrationAccountAgreement> 
ListIntegrationAccountAgreements(....)
{
  var compositeList = new List<IntegrationAccountAgreement>();
  var firstPage = this.LogicManagementClient.
                  IntegrationAccountAgreements.List(....);

  if (firstPage != null)
  {
    compositeList.AddRange(firstPage);
  }

  if (!string.IsNullOrEmpty(firstPage.NextPageLink))  // <=
  {
    ....
  }
  ....
}

Un altro errore evidente. La primaPagina la variabile è usata in modo non sicuro nonostante il fatto che in precedenza nel codice questa variabile sia già utilizzata essendo verificata in via preliminare per null .

Ho trovato ancora più avvisi V3125 nel codice di Azure PowerShell rispetto a quelli V3095 descritti sopra. Tutti loro sono anche dello stesso tipo. Penso che due di quelli che abbiamo considerato siano sufficienti.

V3137 La variabile 'apiVersionSetId' viene assegnata ma non viene utilizzata alla fine della funzione. GetAzureApiManagementApiVersionSet.cs 69 (ApiManagement)

public String ApiVersionSetId { get; set; }
....
public override void ExecuteApiManagementCmdlet()
{
  ....
  string apiVersionSetId;

  if (ParameterSetName.Equals(ContextParameterSet))
  {
    ....
    apiVersionSetId = ApiVersionSetId;
  }
  else
  {
    apiVersionSetId = ....;
  }

  if (string.IsNullOrEmpty(ApiVersionSetId))  // <=
  {
    WriteObject(....);
  }
  else
  {
    WriteObject(Client.GetApiVersionSet(...., ApiVersionSetId))  // <=
  }
}

L'analizzatore segnala che apiVersionSetId la variabile locale è stata inizializzata, ma non utilizzata in alcun modo. Spesso questo schema indica un errore. Penso che in questo caso sia molto probabilmente un errore, soprattutto tenendo conto del fatto che il nome di apiVersionSetId variabile locale e il nome di ApiVersionSetId proprietà differiscono solo per il caso della prima lettera. Dai un'occhiata al codice. Dopo aver inizializzato apiVersionSetId (in un modo o nell'altro), solo ApiVersionSetId la proprietà viene utilizzata ulteriormente nel codice. Sembra estremamente sospetto.

V3137 La variabile 'cacheId' è assegnata ma non viene utilizzata alla fine della funzione. RemoveAzureApiManagementCache.cs 94 (ApiManagement)

public String CacheId { get; set; }
....
public override void ExecuteApiManagementCmdlet()
{
  ....
  string cacheId;

  if (....)
  {
    ....
    cacheId = InputObject.CacheId;
  }
  else if (....)
  {
    ....
    cacheId = cache.CacheId;
  }
  else
  {
    ....
    cacheId = CacheId;
  }
  var actionDescription = string.Format(...., CacheId);   // <=
  var actionWarning = string.Format(...., CacheId);       // <=
  ....
  Client.CacheRemove(resourceGroupName, serviceName, CacheId);  // <=
  ....  
}

Questo è il caso che è quasi lo stesso di quello descritto in precedenza. L'ID cache la variabile locale non viene utilizzata in alcun modo dopo l'inizializzazione. Invece, un'altra proprietà con un nome molto simile CacheId viene usato. Non lo so per certo, potrebbe essere solo un modello di programmazione degli sviluppatori di Azure PowerShell. Ad ogni modo, sembra un errore.

V3143 Il parametro 'valore' viene riscritto all'interno di un setter di proprietà e non viene più utilizzato in seguito. NewAzureIntegrationAccountPartnerCommand.cs 67 (LogicApp)

[Parameter(Mandatory = false, 
  HelpMessage = "The integration account partner type.",
  ValueFromPipelineByPropertyName = false)]
[ValidateSet("B2B", IgnoreCase = false)]
[ValidateNotNullOrEmpty]
public string PartnerType
{
  get { return this.partnerType; }
  set { value = this.partnerType; }  // <=
}

Il tipo di partner campo è dichiarato nel modo seguente:

/// <summary>
/// Default partner type.
/// </summary>
private string partnerType = "B2B";

Nonostante il nome della soluzione (LogicApp) in cui è stato rilevato un errore, non trovo logica in essa. Modifica del valore nel setter non è un evento raro, ma in questo caso si tratta di una perdita del valore originario. Sembra strano. Nella proprietà del codice viene letta una sola volta. Forse, dobbiamo chiedere di nuovo il parere di esperti. Forse proprio non capisco. Il punto è che mi sono imbattuto in diversi schemi uguali:

  • V3143 Il parametro 'value' viene riscritto all'interno di un setter di proprietà e successivamente non viene più utilizzato. NewAzureIntegrationAccountSchemaCommand.cs 79 (LogicApp)
  • V3143 Il parametro 'value' viene riscritto all'interno di un setter di proprietà e successivamente non viene più utilizzato. NewAzureIntegrationAccountSchemaCommand.cs 87 (LogicApp)
  • V3143 Il parametro 'value' viene riscritto all'interno di un setter di proprietà e successivamente non viene più utilizzato. UpdateAzureIntegrationAccountPartnerCommand.cs 67 (LogicApp)
  • V3143 Il parametro 'value' viene riscritto all'interno di un setter di proprietà e successivamente non viene più utilizzato. UpdateAzureIntegrationAccountSchemaCommand.cs 80 (LogicApp)
  • V3143 Il parametro 'value' viene riscritto all'interno di un setter di proprietà e successivamente non viene più utilizzato. UpdateAzureIntegrationAccountSchemaCommand.cs 88 (LogicApp)

Conclusione

Questi sono tutti bug interessanti che sono stati trovati nel codice di Azure PowerShell. Gli appassionati e coloro che sono interessati sono invitati a rivedere gli errori in questo (o qualsiasi altro) progetto. Probabilmente potrei perdere qualcosa di insolito. Per fare la revisione, devi solo scaricare e installare PVS-Studio.

Grazie per aver letto fino alla fine. E, naturalmente, codice senza bug per tutti!