Ma connaissance du SDK Open XML a commencé lorsque j'avais besoin d'une bibliothèque pour créer des documents Word avec des rapports. Après avoir travaillé avec l'API Word pendant plus de 7 ans, je voulais essayer quelque chose de nouveau et de plus pratique. C'est ainsi que j'ai découvert que Microsoft avait une solution alternative. Par tradition, nous pré-vérifions les programmes et bibliothèques utilisés dans l'entreprise à l'aide de l'analyseur PVS-Studio.
introduction
Office Open XML, également connu sous le nom d'OpenXML ou OOXML, est un format basé sur XML pour les documents bureautiques, y compris les documents de traitement de texte, les feuilles de calcul, les présentations et les diagrammes, formes et autres graphiques. La spécification a été développée par Microsoft et adoptée par ECMA International en 2006. En juin 2014, Microsoft a publié le SDK Open XML en open source. La source est désormais disponible sur GitHub sous la licence MIT.
Pour trouver des erreurs dans le code source de la bibliothèque, nous avons utilisé PVS-Studio . C'est un outil de détection de bogues et de vulnérabilités potentielles dans le code source des programmes écrits en C, C ++, C # et Java. Fonctionne dans les systèmes 64 bits sous Windows, Linux et macOS.
Le projet est assez petit et il y a eu peu d'avertissements. Mais le choix de l'image de titre était précisément basé sur les résultats. Il y a beaucoup d'opérateurs conditionnels inutiles dans le code. Il me semble que si vous refactorisez tous ces endroits dans le code, le volume sera sensiblement réduit. En conséquence, la lisibilité du code augmentera également.
Pourquoi l'API Word et non le SDK Open XML
Comme vous l'avez peut-être deviné d'après le titre, j'ai continué à utiliser l'API Word. Cette méthode présente de nombreux inconvénients:
- Ancienne API maladroite;
- Microsoft Office doit être installé;
- La nécessité de distribuer un kit de distribution avec les bibliothèques Office;
- Dépendance de l'API Word sur les paramètres régionaux du système;
- Faible vitesse de travail.
Avec le lieu en général, un incident amusant s'est produit. Windows a une douzaine de paramètres régionaux. Sur l'un des ordinateurs serveurs, il y avait un désordre des États-Unis et du Royaume-Uni. Pour cette raison, des documents Word ont été créés, où au lieu du symbole dollar, il y avait un rouble et les livres n'étaient pas du tout affichées. L'amélioration des paramètres du système d'exploitation a résolu le problème.
En listant tout cela, je me suis demandé à nouveau pourquoi je l'utilise encore ...
Mais non, j'aime toujours mieux l'API Word, et voici pourquoi.
OOXML ressemble à ceci:
<?xml version="1.0" encoding="utf-8" standalone="yes"?>
<w:document ....>
<w:body>
<w:p w:rsidR="00E22EB6"
w:rsidRDefault="00E22EB6">
<w:r>
<w:t>This is a paragraph.</w:t>
</w:r>
</w:p>
<w:p w:rsidR="00E22EB6"
w:rsidRDefault="00E22EB6">
<w:r>
<w:t>This is another paragraph.</w:t>
</w:r>
</w:p>
</w:body>
</w:document>Où <w: r> (Word Run) n'est pas une phrase, ni même un mot, mais tout morceau de texte dont les attributs diffèrent des fragments de texte adjacents.
Il est programmé avec quelque chose comme ceci:
Paragraph para = body.AppendChild(new Paragraph());
Run run = para.AppendChild(new Run());
run.AppendChild(new Text(txt));Le document a une structure interne spécifique et dans le code, vous devez créer les mêmes éléments. Le SDK Open XML, à mon avis, n'a pas suffisamment de couche d'accès aux données abstraites. La création d'un document à l'aide de l'API Word sera plus claire et plus courte. Surtout lorsqu'il s'agit de tables et d'autres structures de données complexes.
À son tour, le SDK Open XML résout un large éventail de tâches. Avec lui, vous pouvez créer des documents non seulement pour Word, mais également pour Excel et PowerPoint. Cette bibliothèque est probablement plus adaptée à certaines tâches, mais j'ai décidé de rester sur l'API Word pour le moment. Dans tous les cas, il ne sera pas possible de l'abandonner complètement, car pour les besoins internes, nous développons un plugin pour Word, et là il est possible d'utiliser uniquement l'API Word.
Deux valeurs pour la chaîne
V3008 La variable '_rawOuterXml' se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 164, 161. OpenXmlElement.cs 164
internal string RawOuterXml
{
get => _rawOuterXml;
set
{
if (string.IsNullOrEmpty(value))
{
_rawOuterXml = string.Empty;
}
_rawOuterXml = value;
}
}Le type chaîne peut avoir 2 types de valeurs: null et valeur textuelle. Il est certainement plus sûr d'utiliser une signification textuelle, mais les deux approches sont valables. Dans ce projet , il est inacceptable d'utiliser la valeur null et elle est réécrite en string.Empty ... du moins c'est comme ça que c'était prévu. Mais en raison d'une erreur dans RawOuterXml , vous pouvez toujours écrire null , puis faire référence à ce champ, en recevant une NullReferenceException .
L' expression V3022 'namespaceUri! = Null' est toujours vraie. OpenXmlElement.cs 497
public OpenXmlAttribute GetAttribute(string localName, string namespaceUri)
{
....
if (namespaceUri == null)
{
// treat null string as empty.
namespaceUri = string.Empty;
}
....
if (HasAttributes)
{
if (namespaceUri != null) // <=
{
....
}
....
}
....
}Cet extrait de code utilise la même approche, l'auteur du code n'a commis aucune erreur majeure, mais sent toujours un mauvais refactoring. Très probablement, une vérification peut être supprimée ici, ce qui réduira la largeur du code et, par conséquent, augmentera sa lisibilité.
À propos de la compacité du code
V3009 Il est étrange que cette méthode renvoie toujours une seule et même valeur de '".xml"'. CustomXmlPartTypeInfo.cs 31
internal static string GetTargetExtension(CustomXmlPartType partType)
{
switch (partType)
{
case CustomXmlPartType.AdditionalCharacteristics:
return ".xml";
case CustomXmlPartType.Bibliography:
return ".xml";
case CustomXmlPartType.CustomXml:
return ".xml";
case CustomXmlPartType.InkContent:
return ".xml";
default:
return ".xml";
}
}Je ne sais pas s'il y a une faute de frappe ici ou si l'auteur du code a écrit du code "sympa" à son avis. Je suis sûr qu'il ne sert à rien de renvoyer autant de valeurs du même type à partir d'une fonction, et le code peut être considérablement réduit.
Ce n'est pas le seul endroit. Voici quelques autres de ces avertissements:
- V3009 Il est étrange que cette méthode renvoie toujours une seule et même valeur de '".xml"'. CustomPropertyPartTypeInfo.cs 25
- V3009 Il est étrange que cette méthode renvoie toujours une seule et même valeur de '".bin"'. EmbeddedControlPersistenceBinaryDataPartTypeInfo.cs 22
Il serait intéressant d'entendre pourquoi écrire comme ça.
V3139 Deux ou plusieurs branches de cas exécutent les mêmes actions. OpenXmlPartReader.cs 560
private void InnerSkip()
{
Debug.Assert(_xmlReader != null);
switch (_elementState)
{
case ElementState.Null:
ThrowIfNull();
break;
case ElementState.EOF:
return;
case ElementState.Start:
_xmlReader.Skip();
_elementStack.Pop();
GetElementInformation();
return;
case ElementState.End:
case ElementState.MiscNode:
// cursor is end element, pop stack
_xmlReader.Skip();
_elementStack.Pop();
GetElementInformation();
return;
....
}
....
}Il y a moins de questions sur ce code. Très probablement, des cas identiques peuvent être combinés et le code deviendra plus court et plus évident.
Quelques autres endroits de ce genre:
- V3139 Deux ou plusieurs branches de cas exécutent les mêmes actions. OpenXmlMiscNode.cs 312
- V3139 Deux ou plusieurs branches de cas exécutent les mêmes actions. CustomPropertyPartTypeInfo.cs 30
- V3139 Deux ou plusieurs branches de cas exécutent les mêmes actions. CustomXmlPartTypeInfo.cs 15
- V3139 Deux ou plusieurs branches de cas exécutent les mêmes actions. OpenXmlElement.cs 1803
Ceux toujours vrai / faux
Il est maintenant temps de fournir quelques exemples de code qui ont déterminé mon choix d'image de titre.
Avertissement 1
L' expression V3022 «Complète ()» est toujours fausse. ParticleCollection.cs 243
private bool IsComplete => Current is null ||
Current == _collection._element.FirstChild;
public bool MoveNext()
{
....
if (IsComplete)
{
return Complete();
}
if (....)
{
return Complete();
}
return IsComplete ? Complete() : true;
}La propriété IsComplete est utilisée 2 fois et il est facile de comprendre à partir du code que sa valeur ne changera pas. Ainsi, à la fin de la fonction, vous pouvez simplement renvoyer la deuxième valeur de l'opérateur ternaire - true .
Avertissement 2
L' expression V3022 '_elementStack.Count> 0' est toujours vraie. OpenXmlDomReader.cs 501
private readonly Stack<OpenXmlElement> _elementStack;
private bool MoveToNextSibling()
{
....
if (_elementStack.Count == 0)
{
_elementState = ElementState.EOF;
return false;
}
....
if (_elementStack.Count > 0) // <=
{
_elementState = ElementState.End;
}
else
{
// no more element, EOF
_elementState = ElementState.EOF;
}
....
}De toute évidence, s'il n'y a pas 0 éléments dans le _elementStack , alors il y en a plus. Le code peut être raccourci d'au moins 8 lignes.
Avertissement 3
L' expression V3022 'rootElement == null' est toujours fausse. OpenXmlPartReader.cs 746
private static OpenXmlElement CreateElement(string namespaceUri, string name)
{
if (string.IsNullOrEmpty(name))
{
throw new ArgumentException(....);
}
if (NamespaceIdMap.TryGetNamespaceId(namespaceUri, out byte nsId)
&& ElementLookup.Parts.Create(nsId, name) is OpenXmlElement element)
{
return element;
}
return new OpenXmlUnknownElement();
}
private bool ReadRoot()
{
....
var rootElement = CreateElement(....);
if (rootElement == null) // <=
{
throw new InvalidDataException(....);
}
....
}La fonction CreateElement ne peut pas renvoyer null . Si l'entreprise a établi une règle pour écrire des méthodes pour créer des nœuds xml qui renvoient un objet valide ou lancent une exception, les utilisateurs de ces fonctions n'ont pas besoin d'abuser des vérifications supplémentaires.
Avertissement 4
L' expression V3022 'nameProvider' n'est toujours pas nulle. L'opérateur '?.' est excessif. OpenXmlSimpleTypeExtensions.cs 50
public static XmlQualifiedName GetSimpleTypeQualifiedName(....)
{
foreach (var validator in validators)
{
if (validator is INameProvider nameProvider &&
nameProvider?.QName is XmlQualifiedName qname) // <=
{
return qname;
}
}
return type.GetSimpleTypeQualifiedName();
}L'opérateur is a le modèle suivant:
expr is type varnameSi expr est évaluée à vrai , alors le nomvar objet sera valide et n'a pas besoin d'être comparée à zéro à nouveau , comme cela se fait dans cet extrait de code.
Avertissement 5
Extension de l' expression V3022 == ".xlsx" || extension == ".xlsm" 'est toujours faux. PrésentationDocument.cs 246
public static PresentationDocument CreateFromTemplate(string path)
{
....
string extension = Path.GetExtension(path);
if (extension != ".pptx" && extension != ".pptm" &&
extension != ".potx" && extension != ".potm")
{
throw new ArgumentException("...." + path, nameof(path));
}
using (PresentationDocument template = PresentationDocument.Open(....)
{
PresentationDocument document = (PresentationDocument)template.Clone();
if (extension == ".xlsx" || extension == ".xlsm")
{
return document;
}
....
}
....
}Un code intéressant s'est avéré. Premier auteur Éliminez tous les documents avec les extensions suivantes ne sont pas .pptx , .pptm ,. potx et. potm , puis a décidé de vérifier qu'il n'y avait pas de .xlsx et .xlsm parmi eux . La fonction PresentationDocument est définitivement victime du refactoring.
Avertissement 7
L' expression V3022 «OpenSettings.MarkupCompatibilityProcessSettings == null» est toujours fausse. OpenXmlPackage.cs 661
public MarkupCompatibilityProcessSettings MarkupCompatibilityProcessSettings
{
get
{
if (_mcSettings is null)
{
_mcSettings = new MarkupCompatibilityProcessSettings(....);
}
return _mcSettings;
}
set
{
_mcSettings = value;
}
}
public MarkupCompatibilityProcessSettings MarkupCompatibilityProcessSettings
{
get
{
if (OpenSettings.MarkupCompatibilityProcessSettings == null) // <=
{
return new MarkupCompatibilityProcessSettings(....);
}
else
{
return OpenSettings.MarkupCompatibilityProcessSettings;
}
}
}La propriété MarkupCompatibilityProcessSettings ne renvoie jamais null . S'il s'avère dans le getter que le champ de classe est nul , alors l'objet est écrasé par un nouveau. Notez également qu'il ne s'agit pas d'un appel récursif d'une propriété, mais de propriétés du même nom provenant de différentes classes. Peut-être qu'une certaine confusion a conduit à la rédaction de chèques inutiles.
Autres avertissements
Avertissement 1
V3080 Déréférencement nul possible. Pensez à inspecter «previousSibling». OpenXmlCompositeElement.cs 380
public OpenXmlElement PreviousSibling()
{
if (!(Parent is OpenXmlCompositeElement parent))
{
return null;
}
....
}
public override T InsertBefore<T>(T newChild, OpenXmlElement referenceChild)
{
....
OpenXmlElement previousSibling = nextNode.PreviousSibling();
prevNode.Next = nextNode;
previousSibling.Next = prevNode; // <=
....
}Et voici un exemple où une vérification supplémentaire ne suffit pas. La méthode PreviousSibling peut retourner null , et le résultat de cette fonction est immédiatement utilisé sans vérification.
2 endroits plus dangereux:
- V3080 Déréférencement nul possible. Pensez à inspecter «prevNode». OpenXmlCompositeElement.cs 489
- V3080 Déréférencement nul possible. Pensez à inspecter «prevNode». OpenXmlCompositeElement.cs 497
Avertissement 2
V3093 L'opérateur '&' évalue les deux opérandes. Peut-être qu'un opérateur de court-circuit «&&» devrait être utilisé à la place. UniqueAttributeValueConstraint.cs 60
public override ValidationErrorInfo ValidateCore(ValidationContext context)
{
....
foreach (var e in root.Descendants(....))
{
if (e != element & e.GetType() == elementType) // <=
{
var eValue = e.ParsedState.Attributes[_attribute];
if (eValue.HasValue && _comparer.Equals(....))
{
return true;
}
}
}
....
}Certaines personnes aiment appliquer l'opérateur «&» aux expressions logiques là où elles ne le devraient pas. Dans le cas de cet opérateur, le deuxième opérande est évalué en premier, quel que soit le résultat du premier. Ce n'est pas une erreur très grave ici, mais un tel code bâclé après refactoring peut conduire à des exceptions potentielles NullReferenceException .
Avertissement 3
V3097 Exception possible: le type marqué par [Serializable] contient des membres non sérialisables non marqués par [NonSerialized]. OpenXmlPackageValidationEventArgs.cs 15
[Serializable]
[Obsolete(ObsoleteAttributeMessages.ObsoleteV1ValidationFunctionality, false)]
[EditorBrowsable(EditorBrowsableState.Never)]
public sealed class OpenXmlPackageValidationEventArgs : EventArgs
{
private string _message;
[NonSerialized]
private readonly object _sender;
[NonSerialized]
private OpenXmlPart _subPart;
[NonSerialized]
private OpenXmlPart _part;
....
internal DataPartReferenceRelationship
DataPartReferenceRelationship { get; set; } // <=
}La sérialisation de la classe OpenXmlPackageValidationEventArgs peut échouer car l'une des propriétés a été oubliée pour être marquée comme non sérialisable. Ou vous devez modifier le type de retour de cette propriété pour qu'il soit sérialisable, sinon une exception peut se produire lors de l'exécution.
Conclusion
Dans l'entreprise, nous aimons les projets et les technologies Microsoft. Dans la section où nous listons les projets Open Source testés avec PVS-Studio, nous avons même alloué une section séparée pour Microsoft. Il y a déjà accumulé 21 projets, sur lesquels 26 articles ont été écrits. C'est le 27.
Je suis sûr que vous vous demandez s'il y a Microsoft parmi nos clients. La réponse est oui! Mais n'oublions pas qu'il s'agit d'une énorme société leader du développement dans le monde entier. Il y a certainement des divisions qui utilisent déjà PVS-Studio dans leurs projets, mais il y en a encore plus qui ne l'utilisent pas! Et notre expérience avec les projets open source montre qu'il leur manque clairement un bon outil pour trouver des bogues;).
Plus de nouvelles de l'actualité pour ceux qui s'intéressent à l'analyse du code en C ++, C # et Java: nous avons récemment ajouté le support du standard OWASP et augmentons activement sa couverture.
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien de traduction: Svyatoslav Razmyslov. Analyse de la qualité du code du SDK Open XML de Microsoft .