Este es un segundo artículo, que se centra en el uso del analizador PVS-Studio en sistemas CI en la nube. Esta vez consideraremos la plataforma Azure DevOps, una solución de CI\CD en la nube de Microsoft. Estaremos analizando el proyecto ShareX.
Para obtener información actualizada al respecto, siga la página de documentación actualizada "Uso con Azure DevOps".
Necesitaremos tres componentes. El primero es el analizador PVS-Studio. El segundo es Azure DevOps, con el que integraremos el analizador. El tercero es el proyecto que revisaremos para demostrar las capacidades de PVS-Studio cuando se trabaja en la nube. Así que pongámonos en marcha.
PVS-Studio es un analizador de código estático para encontrar errores y defectos de seguridad. La herramienta admite el análisis de código C, C++ y C#.
Azure DevOps. La plataforma Azure DevOps incluye herramientas como Azure Pipeline, Azure Board, Azure Artifacts y otras que aceleran el proceso de creación de software y mejoran su calidad.
ShareX es una aplicación gratuita que te permite capturar y grabar cualquier parte de la pantalla. El proyecto está escrito en C# y es eminentemente adecuado para mostrar la configuración del lanzamiento del analizador estático. El código fuente del proyecto está disponible en GitHub.
La salida del comando cloc para el proyecto ShareX:
| Idioma | archivos | en blanco | comentario | Código |
|---|---|---|---|---|
| C# | 696 | 20658 | 24423 | 102565 |
| Guión de MSBuild | 11 | 1 | 77 | 5859 |
En otras palabras, el proyecto es pequeño, pero suficiente para demostrar el trabajo de PVS-Studio junto con la plataforma en la nube.
Comencemos la configuración
Para comenzar a trabajar en Azure DevOps, sigamos el enlace y presionemos "Comenzar gratis con GitHub".
Otorgue a la aplicación de Microsoft acceso a los datos de la cuenta de GitHub.
Tendrás que crear una cuenta de Microsoft para completar tu registro.
Después del registro, cree un proyecto:
A continuación, debemos movernos a "Pipelines" - "Builds" y crear un nuevo Build pipeline.
Cuando se nos pregunte dónde se encuentra nuestro código, responderemos:GitHub.
Autoriza Azure Pipelines y elige el repositorio con el proyecto, para lo cual configuraremos la ejecución del analizador estático.
En la ventana de selección de plantillas, elija "Canalización inicial".
Podemos ejecutar análisis de código estático del proyecto de dos maneras:utilizando agentes alojados por Microsoft o autohospedados.
Primero, usaremos agentes hospedados por Microsoft. Dichos agentes son máquinas virtuales ordinarias que se inician cuando ejecutamos nuestra canalización. Se eliminan cuando se realiza la tarea. El uso de dichos agentes nos permite no perder tiempo en su soporte y actualización, pero impone ciertas restricciones, por ejemplo, la incapacidad de instalar software adicional que se usa para construir un proyecto.
Reemplacemos la configuración predeterminada sugerida por la siguiente para usar agentes hospedados por Microsoft:
# Setting up run triggers
# Run only for changes in the master branch
trigger:
- master
# Since the installation of random software in virtual machines
# is prohibited, we'll use a Docker container,
# launched on a virtual machine with Windows Server 1803
pool:
vmImage: 'win1803'
container: microsoft/dotnet-framework:4.7.2-sdk-windowsservercore-1803
steps:
# Download the analyzer distribution
- task: PowerShell@2
inputs:
targetType: 'inline'
script: 'Invoke-WebRequest
-Uri https://files.pvs-studio.com/PVS-Studio_setup.exe
-OutFile PVS-Studio_setup.exe'
- task: CmdLine@2
inputs:
workingDirectory: $(System.DefaultWorkingDirectory)
script: |
# Restore the project and download dependencies
nuget restore .\ShareX.sln
# Create the directory, where files with analyzer reports will be saved
md .\PVSTestResults
# Install the analyzer
PVS-Studio_setup.exe /VERYSILENT /SUPPRESSMSGBOXES
/NORESTART /COMPONENTS=Core
# Create the file with configuration and license information
"C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe"
credentials
-u $(PVS_USERNAME)
-n $(PVS_KEY)
# Run the static analyzer and convert the report in html.
"C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe"
-t .\ShareX.sln
-o .\PVSTestResults\ShareX.plog
"C:\Program Files (x86)\PVS-Studio\PlogConverter.exe"
-t html
-o .\PVSTestResults\
.\PVSTestResults\ShareX.plog
# Save analyzer reports
- task: PublishBuildArtifacts@1
inputs:
pathToPublish: PVSTestResults
artifactName: PVSTestResults
Guardemos la canalización y creemos variables que se utilizarán para crear el archivo de licencia. Para hacer esto, abra la ventana de edición de canalización y haga clic en "Variables" en la esquina superior derecha.
Luego, agregue dos variables - PVS_USERNAME y PVS_KEY , que contiene el nombre de usuario y la clave de licencia respectivamente. Al crear la PVS_KEY variable no olvide seleccionar "Mantener este valor en secreto" para cifrar los valores de la variable con una clave RSA de 2048 bits y suprimir la salida del valor de la variable en el registro de rendimiento de la tarea.
Guarde las variables y ejecute la canalización haciendo clic en "Ejecutar".
La segunda opción para ejecutar el análisis:usar un agente autohospedado. Podemos personalizar y administrar nosotros mismos los agentes autohospedados. Dichos agentes brindan más oportunidades para instalar software, necesario para construir y probar nuestro producto de software.
Antes de usar dichos agentes, debe configurarlos de acuerdo con las instrucciones e instalar y configurar el analizador estático.
Para ejecutar la tarea en un agente autohospedado, reemplazaremos la configuración sugerida con lo siguiente:
# Setting up triggers
# Run the analysis for master-branch
trigger:
- master
# The task is run on a self-hosted agent from the pool 'MyPool'
pool: 'MyPool'
steps:
- task: CmdLine@2
inputs:
workingDirectory: $(System.DefaultWorkingDirectory)
script: |
# Restore the project and download dependencies
nuget restore .\ShareX.sln
# Create the directory where files with analyzer reports will be saved
md .\PVSTestResults
# Run the static analyzer and convert the report in html.
"C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe"
-t .\ShareX.sln
-o .\PVSTestResults\ShareX.plog
"C:\Program Files (x86)\PVS-Studio\PlogConverter.exe"
-t html
-o .\PVSTestResults\
.\PVSTestResults\ShareX.plog
# Save analyzer reports
- task: PublishBuildArtifacts@1
inputs:
pathToPublish: PVSTestResults
artifactName: PVSTestResultsUna vez completada la tarea, puede descargar el archivo con los informes del analizador en la pestaña "Resumen" o puede usar la extensión Enviar correo que permite configurar el envío de correo electrónico o considerar otra herramienta conveniente en Marketplace.
Resultados del análisis
Ahora veamos algunos errores encontrados en el proyecto probado, ShareX.
Comprobaciones excesivas
Para calentar, comencemos con fallas simples en el código, es decir, con comprobaciones redundantes:
private void PbThumbnail_MouseMove(object sender, MouseEventArgs e)
{
....
IDataObject dataObject
= new DataObject(DataFormats.FileDrop,
new string[] { Task.Info.FilePath });
if (dataObject != null)
{
Program.MainForm.AllowDrop = false;
dragBoxFromMouseDown = Rectangle.Empty;
pbThumbnail.DoDragDrop(dataObject,
DragDropEffects.Copy | DragDropEffects.Move);
Program.MainForm.AllowDrop = true;
}
....
}Advertencia de PVS-Studio: V3022 [CWE-571] La expresión 'dataObject!=null' siempre es verdadera. TaskThumbnailPanel.cs 415
Prestemos atención a la comprobación del dataObject variable para null . ¿Por qué está aquí? objeto de datos no puede ser null en este caso, ya que se inicializa mediante una referencia en un objeto creado. Como resultado, tenemos un control excesivo. ¿Crítico? No. ¿Parece sucinto? No. Claramente, es mejor eliminar esta verificación para no saturar el código.
Veamos otro fragmento de código que podemos comentar de manera similar:
private static Image GetDIBImage(MemoryStream ms)
{
....
try
{
....
return new Bitmap(bmp);
....
}
finally
{
if (gcHandle != IntPtr.Zero)
{
GCHandle.FromIntPtr(gcHandle).Free();
}
}
....
}
private static Image GetImageAlternative()
{
....
using (MemoryStream ms = dataObject.GetData(format) as MemoryStream)
{
if (ms != null)
{
try
{
Image img = GetDIBImage(ms);
if (img != null)
{
return img;
}
}
catch (Exception e)
{
DebugHelper.WriteException(e);
}
}
}
....
}Advertencia de PVS-Studio: V3022 [CWE-571] La expresión 'img !=null' siempre es verdadera. ClipboardHelpers.cs 289
En GetImageAlternative método, el img se comprueba que la variable no sea nula justo después de una nueva instancia del mapa de bits se crea la clase. La diferencia con el ejemplo anterior aquí es que usamos el GetDIBImage método en lugar del constructor para inicializar el img variable. El autor del código sugiere que podría ocurrir una excepción en este método, pero declara solo bloques intentar y finalmente , omitiendo capturar . Por lo tanto, si ocurre una excepción, el método de llamada GetImageAlternative no obtendrá una referencia a un objeto del mapa de bits type, pero tendrá que manejar la excepción en su propio catch bloquear. En este caso, el img la variable no se inicializará y el hilo de ejecución ni siquiera alcanzará el img !=null check pero entrará en el bloque catch. En consecuencia, el analizador señaló una verificación excesiva.
Consideremos el siguiente ejemplo de una advertencia V3022:
private void btnCopyLink_Click(object sender, EventArgs e)
{
....
if (lvClipboardFormats.SelectedItems.Count == 0)
{
url = lvClipboardFormats.Items[0].SubItems[1].Text;
}
else if (lvClipboardFormats.SelectedItems.Count > 0)
{
url = lvClipboardFormats.SelectedItems[0].SubItems[1].Text;
}
....
}Advertencia de PVS-Studio: V3022 [CWE-571] La expresión 'lvClipboardFormats.SelectedItems.Count> 0' siempre es verdadera. AfterUploadForm.cs 155
Echemos un vistazo más de cerca a la segunda expresión condicional. Allí comprobamos el valor del Count de sólo lectura propiedad. Esta propiedad muestra el número de elementos en la instancia de la colección SelectedItems . La condición solo se ejecuta si Count propiedad es mayor que cero. Todo estaría bien, pero en el si externo sentencia Recuento ya está marcado como 0. La instancia de SelectedItems la colección no puede tener un número de elementos menor que cero, por lo tanto, Count es igual o mayor que 0. Dado que ya hemos realizado el Cuenta verifique 0 en el primer if declaración y era falsa, no tiene sentido escribir otro Cuenta comprobar si es mayor que cero en la otra rama.
El ejemplo final de una advertencia V3022 será el siguiente fragmento de código:
private void DrawCursorGraphics(Graphics g)
{
....
int cursorOffsetX = 10, cursorOffsetY = 10, itemGap = 10, itemCount = 0;
Size totalSize = Size.Empty;
int magnifierPosition = 0;
Bitmap magnifier = null;
if (Options.ShowMagnifier)
{
if (itemCount > 0) totalSize.Height += itemGap;
....
}
....
}Advertencia de PVS-Studio: V3022 La expresión 'itemCount> 0' siempre es falsa. RegionCaptureForm.cs 1100
El analizador notó que la condición itemCount > 0 siempre será falso, ya que itemCount se declara la variable y al mismo tiempo se le asigna cero arriba. Esta variable no se usa en ninguna parte hasta la condición misma, por lo tanto, el analizador tenía razón sobre la expresión condicional, cuyo valor siempre es falso.
Bueno, ahora veamos algo realmente sápido.
La mejor manera de entender un error es visualizarlo
Nos parece que se encontró un error bastante interesante en este lugar:
public static void Pixelate(Bitmap bmp, int pixelSize)
{
....
float r = 0, g = 0, b = 0, a = 0;
float weightedCount = 0;
for (int y2 = y; y2 < yLimit; y2++)
{
for (int x2 = x; x2 < xLimit; x2++)
{
ColorBgra color = unsafeBitmap.GetPixel(x2, y2);
float pixelWeight = color.Alpha / 255;
r += color.Red * pixelWeight;
g += color.Green * pixelWeight;
b += color.Blue * pixelWeight;
a += color.Alpha * pixelWeight;
weightedCount += pixelWeight;
}
}
....
ColorBgra averageColor = new ColorBgra((byte)(b / weightedCount),
(byte)(g / weightedCount), (byte)(r / weightedCount),
(byte)(a / pixelCount));
....
}No me gustaría mostrar todas las cartas y revelar lo que ha encontrado nuestro analizador, así que dejémoslo a un lado por un momento.
Por el nombre del método, es fácil adivinar lo que está haciendo:le das una imagen o un fragmento de una imagen y lo pixela. El código del método es bastante largo, por lo que no lo citaremos en su totalidad, solo trataremos de explicar su algoritmo y explicar qué tipo de error logró encontrar PVS-Studio.
Este método recibe dos parámetros:un objeto del Bitmap tipo y el valor de int tipo que indica el tamaño de la pixelación. El algoritmo de operación es bastante simple:
1) Divida el fragmento de imagen recibido en cuadrados con el lado igual al tamaño de la pixelación. Por ejemplo, si tenemos el tamaño de pixelado igual a 15, obtendremos un cuadrado que contiene 15x15=225 píxeles.
2) Además, recorremos cada píxel en este cuadrado y acumulamos los valores de los campos Rojo , Verde , Azul y Alfa en variables intermedias, y antes multiplicar el valor del color correspondiente y el canal alfa por el pixelWeight variable, obtenida al dividir el Alfa valor por 255 (el Alfa la variable es del byte escribe). Además, al atravesar píxeles sumamos los valores, escritos en pixelWeight en el weightedCount variable. El fragmento de código que ejecuta las acciones anteriores es el siguiente:
ColorBgra color = unsafeBitmap.GetPixel(x2, y2);
float pixelWeight = color.Alpha / 255;
r += color.Red * pixelWeight;
g += color.Green * pixelWeight;
b += color.Blue * pixelWeight;
a += color.Alpha * pixelWeight;
weightedCount += pixelWeight;Por cierto, tenga en cuenta que si el valor de Alpha la variable es cero, pixelWeight no se agregará al weightedCount variable cualquier valor para este píxel. Lo necesitaremos en el futuro.
3) Después de atravesar todos los píxeles en el cuadrado actual, podemos crear un color "promedio" común para este cuadrado. El código que hace esto tiene el siguiente aspecto:
ColorBgra averageColor = new ColorBgra((byte)(b / weightedCount),
(byte)(g / weightedCount), (byte)(r / weightedCount),
(byte)(a / pixelCount));4) Ahora, cuando obtuvimos el color final y lo escribimos en averageColor variable, podemos recorrer de nuevo cada píxel del cuadrado y asignarle un valor de averageColor .
5) Vuelva al punto 2 mientras tenemos cuadrados sin manejar.
Una vez más, el weightedCount variable no es igual al número de todos los píxeles en un cuadrado. Por ejemplo, si una imagen contiene un píxel completamente transparente (valor cero en el canal alfa), el pixelWeight variable será cero para este píxel (0/255 =0). Por lo tanto, este píxel no afectará la formación del weightedCount variable. Es bastante lógico:no tiene sentido tener en cuenta los colores de un píxel completamente transparente.
Entonces todo parece razonable:la pixelación debe funcionar correctamente. Y en realidad lo hace. Eso no se aplica a las imágenes png que incluyen píxeles con valores en el canal alfa por debajo de 255 y distintos de cero. Observe la imagen pixelada a continuación:
¿Has visto la pixelación? Nosotros tampoco. Bien, ahora revelemos esta pequeña intriga y expliquemos dónde se esconde exactamente el error en este método. El error se coló en la línea del pixelWeight cálculo de variables:
float pixelWeight = color.Alpha / 255;El hecho es que al declarar el pixelWeight variable como flotante , el autor del código dio a entender que al dividir el Alpha campo por 255, obtendrá números fraccionarios además de cero y uno. Aquí es donde se esconde el problema, ya que el Alfa la variable es del byte escribe. Al sumergirlo por 255, obtenemos un valor entero. Solo después de eso, se convertirá implícitamente en el flotador tipo, lo que significa que la parte fraccionaria se pierde.
Es fácil explicar por qué es imposible pixelar imágenes png con algo de transparencia. Dado que para estos píxeles los valores del canal alfa están en el rango 0
float pixelWeight = (float)color.Alpha / 255;Bueno, ya es hora de citar el mensaje de PVS-Studio por el código incorrecto:
Advertencia de PVS-Studio: V3041 [CWE-682] La expresión se transformó implícitamente del tipo 'int' al tipo 'float'. Considere utilizar una conversión de tipos explícita para evitar la pérdida de una parte fraccionaria. Un ejemplo:doble A =(doble)(X) / Y;. ImageHelpers.cs 1119
A modo de comparación, citemos la captura de pantalla de una imagen verdaderamente pixelada, obtenida en la versión de la aplicación corregida:
Posible excepción de referencia nula
public static bool AddMetadata(Image img, int id, string text)
{
....
pi.Value = bytesText;
if (pi != null)
{
img.SetPropertyItem(pi);
return true;
}
....
}Advertencia de PVS-Studio: V3095 [CWE-476] El objeto 'pi' se usó antes de que se verificara contra nulo. Verifique las líneas:801, 803. ImageHelpers.cs 801
Este fragmento de código muestra que el autor esperaba que el pi la variable puede ser null , por eso antes de llamar al método SetPropertyItem , la comprobación pi !=null tiene lugar Es extraño que antes de esta verificación, a la propiedad se le asigne una matriz de bytes, porque si pi es nulo , una excepción de NullReferenceException se lanzará el tipo.
Una situación similar se ha notado en otro lugar:
private static void Task_TaskCompleted(WorkerTask task)
{
....
task.KeepImage = false;
if (task != null)
{
if (task.RequestSettingUpdate)
{
Program.MainForm.UpdateCheckStates();
}
....
}
....
}Advertencia de PVS-Studio: V3095 [CWE-476] El objeto 'tarea' se usó antes de que se verificara contra nulo. Verificar líneas:268, 270. TaskManager.cs 268
PVS-Studio encontró otro error similar. El punto es el mismo, así que no hay mucha necesidad de citar el fragmento de código, el mensaje del analizador será suficiente.
Advertencia de PVS-Studio: V3095 [CWE-476] El objeto 'Config.PhotobucketAccountInfo' se usó antes de que se verificara contra nulo. Verifique las líneas:216, 219. UploadersConfigForm.cs 216
El mismo valor de retorno
Se encontró un fragmento de código sospechoso en EvalWindows método de WindowsList clase, que devuelve verdadero en todos los casos:
public class WindowsList
{
public List<IntPtr> IgnoreWindows { get; set; }
....
public WindowsList()
{
IgnoreWindows = new List<IntPtr>();
}
public WindowsList(IntPtr ignoreWindow) : this()
{
IgnoreWindows.Add(ignoreWindow);
}
....
private bool EvalWindows(IntPtr hWnd, IntPtr lParam)
{
if (IgnoreWindows.Any(window => hWnd == window))
{
return true; // <=
}
windows.Add(new WindowInfo(hWnd));
return true; // <=
}
}Advertencia de PVS-Studio: V3009 Es extraño que este método siempre devuelva el mismo valor de 'verdadero'. WindowsList.cs 82
Parece lógico que si en la lista llamada IgnoreWindows hay un puntero con el mismo nombre que hWnd , el método debe devolver falso .
El Ignorar Windows la lista se puede completar al llamar al constructor WindowsList(IntPtr ignorar ventana) o directamente accediendo a la propiedad ya que es pública. De todos modos, según Visual Studio, de momento en el código no se rellena esta lista. Este es otro lugar extraño de este método.
Nota. Después de hablar con uno de los desarrolladores de ShareX, descubrimos que el método EvalWindows que siempre devuelve el valor verdadero se escribió intencionalmente de esa manera.
Llamada no segura de controladores de eventos
protected void OnNewsLoaded()
{
if (NewsLoaded != null)
{
NewsLoaded(this, EventArgs.Empty);
}
}Advertencia de PVS-Studio: V3083 [CWE-367] La invocación no segura del evento 'NewsLoaded', NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. NewsListControl.cs 111
Aquí podría ocurrir un caso muy desagradable. Después de comprobar el NewsLoaded variable para nulo, el método, que maneja un evento, se puede cancelar, por ejemplo, en otro subproceso. En este caso, cuando lleguemos al cuerpo de la sentencia if, la variable NewsLoaded ya será nulo. Una Excepción de referencia nula puede ocurrir al intentar llamar a suscriptores desde el evento NewsLoaded , que es nulo. Es mucho más seguro usar un operador condicional nulo y reescribir el código anterior de la siguiente manera:
protected void OnNewsLoaded()
{
NewsLoaded?.Invoke(this, EventArgs.Empty);
}El analizador apuntó a 68 fragmentos similares. No los describiremos a todos, todos tienen un patrón de llamada similar.
Retornar nulo desde ToString
Recientemente, descubrí en un artículo interesante de mi colega que Microsoft no recomienda devolver nulo desde el método anulado ToString . PVS-Studio lo sabe muy bien:
public override string ToString()
{
lock (loggerLock)
{
if (sbMessages != null && sbMessages.Length > 0)
{
return sbMessages.ToString();
}
return null;
}
}Advertencia de PVS-Studio: V3108 No se recomienda devolver 'null' desde el método 'ToSting()'. Registrador.cs 167
¿Por qué se asigna si no se usa?
public SeafileCheckAccInfoResponse GetAccountInfo()
{
string url = URLHelpers.FixPrefix(APIURL);
url = URLHelpers.CombineURL(APIURL, "account/info/?format=json");
....
}Advertencia de PVS-Studio: V3008 A la variable 'url' se le asignan valores dos veces seguidas. Quizás esto sea un error. Verificar líneas:197, 196. Seafile.cs 197
Como podemos ver en el ejemplo, al declarar la url variable, se le asigna un valor, devuelto por el método FixPrefix . En la siguiente línea, borramos el valor obtenido incluso sin usarlo en ningún lado. Obtenemos algo similar al código muerto:funciona, pero no afecta el resultado. Lo más probable es que este error sea el resultado de copiar y pegar, ya que dichos fragmentos de código tienen lugar en 9 métodos más. Como ejemplo, citaremos dos métodos con una primera línea similar:
public bool CheckAuthToken()
{
string url = URLHelpers.FixPrefix(APIURL);
url = URLHelpers.CombineURL(APIURL, "auth/ping/?format=json");
....
}
....
public bool CheckAPIURL()
{
string url = URLHelpers.FixPrefix(APIURL);
url = URLHelpers.CombineURL(APIURL, "ping/?format=json");
....
}Conclusiones
Como podemos ver, la complejidad de la configuración de las comprobaciones automáticas del analizador no depende del sistema CI elegido. Nos tomó literalmente 15 minutos y varios clics del mouse configurar la verificación de nuestro código de proyecto con un analizador estático.
En conclusión, lo invitamos a descargar y probar el analizador en sus proyectos.