Geekly Articles each Day

Umbracoソースコードの再確認

時間は容赦ありません。 C＃コードの静的アナライザーの出力を発表し、最初のプロジェクトをチェックし、それについての記事を書き始めたのはごく最近のようです。 そして今、この瞬間から1年が過ぎました。 アナライザーの特性を改善し、新しい診断ルールを追加し、誤検知の統計を収集し、その原因を排除し、ユーザーと対話し、他の多くの問題を解決するための1年間の骨の折れる複雑な作業。 私たちが自分たちのために選んだ困難であるが信じられないほど興味深い道で、多くの大小の勝利の年。 1年前に新しいC＃アナライザーであるUmbracoを使用した研究のために最初に来たプロジェクトを再テストするときが来ました。


はじめに


Umbracoプロジェクトのエラーの検索は、同僚のAndrei Karpov による記事に捧げられました 。 プロジェクトは今年も継続して開発され、現在、約425 KLOCの拡張子「.cs」を持つ約3340個のファイルが含まれています（Andreyによる検証の時点で、プロジェクトにはそれぞれ拡張子「.cs」および400 KLOCの3200個のファイルが含まれていました）。

Umbracoでの最初のテストでは、比較的少ないエラーが明らかになりましたが、それでもこのことについて記事を書き、新しいC＃アナライザーの作業の品質について最初の結論を出すのに十分興味深いものでした。 Umbracoプロジェクトを再調査し、結果を2015年11月に取得した結果と比較するために、アナライザーが多数の新しい診断ルールと改善されたエラー検索メカニズムを取得したことは、さらに興味深いことです。 検証には、最新バージョンのUmbracoソースコードを使用しました。これは、以前と同様に、 GitHubポータルからダウンロードでき、最新バージョンのPVS-Studio 6.11アナライザーもダウンロードできます。

監査の結果、508件の警告が受信されました。 これらのうち、最初のレベル-71、2番目-358、3番目-79：


同時に、疑わしいサイトの全体的な密度係数（KLOCごとの警告の数）は1.12です。 これは、1,000行のコードにつき約1つの警告に対応する優れた指標です。 しかし、警告は本当の間違いを意味するものではありません。 静的アナライザーの場合、誤検出の一定の割合は正常です。 また、実際のエラーに非常によく似た警告を受け取ることがよくありますが、作成者がコードを調べると、そうではないことがわかります。 時間を節約するために、低レベルの警告は考慮しません。通常、誤検知の割合が高いためです。

PVS-Studioによって発行された警告を分析し、HighおよびMeduimレベルで約56％の誤検知を検出しました。 残りの警告には、コードの実際のエラーだけでなく、詳しく見る価値のある非常に疑わしい構造が含まれています。

2015年のテストと比較して、アナライザーの品質について何が言えますか？ あなたの目を引く最初のもの-前の記事で説明したものからはほとんど警告が見つかりませんでした。 Umbracoプロジェクトの作者がAndreiの記事に注目を集め、そこで引用されているエラーを修正したと信じたいと思います。 もちろん、プロジェクトは継続的に開発されており、日々の作業の過程でエラーが修正される可能性があります。 いずれにせよ-古い間違いはほとんどありません。 しかし、多くの新しいものがあります！ 最も興味深いものをあげます。

検証結果


ゼロによる潜在的な除算

PVS-Studioアナライザーの警告 ： V3064潜在的なゼロ除算。 分母「maxWidthHeight」の検査を検討してください。 ImageHelper.cs 154
PVS-Studioアナライザーの警告 ： V3064潜在的なゼロ除算。 分母「maxWidthHeight」の検査を検討してください。 ImageHelper.cs 155

private static ResizedImage GenerateThumbnail(....) { .... if (maxWidthHeight >= 0) { var fx = (float)image.Size.Width / maxWidthHeight; // <= var fy = (float)image.Size.Height / maxWidthHeight; // <= .... } .... }

上記のコードには、2つのエラーが同時に含まれていますが、2番目のエラーは発生しません。 ifブロックの条件により、変数maxWidthHeightが消滅し、ブロック内で除数として機能します。 一般に、このようなコードは長時間正常に機能する可能性があり、これが危険です。 変数名maxWidthHeightに基づいて、その値はゼロではない可能性が最も高いと結論付けることができます。 しかし、もしそれがどうしたら？ この設計の修正バージョンの形式は次のとおりです。

 private static ResizedImage GenerateThumbnail(....) { .... if (maxWidthHeight > 0) { var fx = (float)image.Size.Width / maxWidthHeight; var fy = (float)image.Size.Height / maxWidthHeight; .... } .... }

変数maxWidthHeightがゼロに等しい場合は、個別に処理する必要があります。

迷惑なタイプミス

PVS-Studioアナライザーの警告 ： V3080 null参照解除の可能性。 「context.Request」の検査を検討してください。 StateHelper.cs 369

 public static bool HasCookies { get { var context = HttpContext; return context != null && context.Request != null & // <= context.Request.Cookies != null && context.Response != null && context.Response.Cookies != null; } }

入力ミス： &&演算子の代わりに、 ＆が使用されました。 context.Request.Cookies！= Null条件は、前のcontext.Request！= Null条件のチェック結果に関係なくチェックされます。 context.Requestが nullの場合、必然的にnull参照アクセスになります 。 この設計の修正バージョンの形式は次のとおりです。

 public static bool HasCookies { get { var context = HttpContext; return context != null && context.Request != null && context.Request.Cookies != null && context.Response != null && context.Response.Cookies != null; } }

タイムリーなヌル等値チェック

PVS-Studioアナライザーの警告 ： V3027変数 'rootDoc'は、同じ論理式でnullに対して検証される前に、論理式で使用されました。 publishRootDocument.cs 34

 public bool Execute(....) { .... if (rootDoc.Text.Trim() == documentName.Trim() && // <= rootDoc != null && rootDoc.ContentType != null) .... }

アクセスにrootDoc.Textを使用した後、 rootDoc変数のnull がチェックされます 。 この設計の修正バージョンの形式は次のとおりです。

 public bool Execute(....) { .... if (rootDoc != null && rootDoc.Text.Trim() == documentName.Trim() && rootDoc.ContentType != null) .... }

文字列内の負の文字インデックス

PVS-Studioアナライザーの警告 ： V3057 ' Substring '関数は、負でない値が期待されているときに '-1'値を受け取る可能性があります。 2番目の引数を調べます。 ContentExtensions.cs 82

 internal static CultureInfo GetCulture(....) { .... var pos = route.IndexOf('/'); domain = pos == 0 ? null : domainHelper.DomainForNode( int.Parse(route.Substring(0, pos)), current) // <= .UmbracoDomain; .... }

ルート行は文字「/」を検索し、その後インデックスがpos変数に割り当てられます。 著者は、行の先頭（ pos == 0）で文字を見つける可能性を考慮しましたが、その不在の確率を考慮しませんでした：この場合、変数posは-1の値を取得します。 これは、次にpos変数を使用してサブストリングroute.Substring（0、pos）を抽出するときに例外をスローします。 この設計の修正バージョンの形式は次のとおりです。

 internal static CultureInfo GetCulture(....) { .... var pos = route.IndexOf('/'); domain = (pos <= 0) ? null : domainHelper.DomainForNode( int.Parse(route.Substring(0, pos)), current) .UmbracoDomain; .... }

同様の警告：


時は金なり

PVS-Studioアナライザーの警告 ： V3057 「DateTime」コンストラクターは「0」値を受け取りますが、正の値が必要です。 2番目の引数を調べます。 DateTimeExtensions.cs 24
PVS-Studioアナライザーの警告 ： V3057 「DateTime」コンストラクターは「0」値を受け取りますが、正の値が必要です。 3番目の引数を調べます。 DateTimeExtensions.cs 24
PVS-Studioアナライザーの警告 ： V3057 「DateTime」コンストラクターは「0」値を受け取りますが、正の値が必要です。 3番目の引数を調べます。 DateTimeExtensions.cs 26

 public static DateTime TruncateTo(this DateTime dt, DateTruncate truncateTo) { if (truncateTo == DateTruncate.Year) return new DateTime(dt.Year, 0, 0); // <= x2 if (truncateTo == DateTruncate.Month) return new DateTime(dt.Year, dt.Month, 0); // <= .... }

この小さなコードには、診断ルールV3057によっても検出された3つのエラーが一度に含まれています。 すべてのエラーは、 DateTimeクラスのオブジェクトの不正な初期化に関連付けられています。そのコンストラクターの形式はpublic DateTime（int year、int month、int day）です。 この場合、パラメーターyear 、 month 、およびdayは値<1を取ることができません。そうでない場合、 ArgumentOutOfRangeExceptionがスローされます。 この設計の修正バージョンの形式は次のとおりです。

 public static DateTime TruncateTo(this DateTime dt, DateTruncate truncateTo) { if (truncateTo == DateTruncate.Year) return new DateTime(dt.Year, 1, 1); if (truncateTo == DateTruncate.Month) return new DateTime(dt.Year, dt.Month, 1); .... }

誤った状態

PVS-Studioアナライザーの警告 ： V3125 nullに対して検証された後、「ct」オブジェクトが使用されました。 行を確認：171、163。ContentTypeControllerBase.cs 171

 protected TContentType PerformPostSave<....>(....) { var ctId = Convert.ToInt32(....); .... if (ctId > 0 && ct == null) throw new HttpResponseException(HttpStatusCode.NotFound); .... if ((....) && (ctId == 0 || ct.Alias != contentTypeSave.Alias)) // <= .... }

このコードフラグメントの条件（ctId> 0 && ct == null）により、null参照を介した後続のアクセスの可能性があります。 条件の両方の部分が満たされた場合にのみ、 HttpResponseExceptionがスローされます。 変数ctIdが<= 0の場合、変数ctの値に関係なく、作業は続行されます。 また、2番目の条件（ ctが使用される）でエラーを修正する必要があります。 この設計の修正バージョンの形式は次のとおりです。

 protected TContentType PerformPostSave<....>(....) { var ctId = Convert.ToInt32(....); .... if (ctId > 0 && ct == null) throw new HttpResponseException(HttpStatusCode.NotFound); .... if ((....) && (ctId == 0 || (ct != null && ct.Alias != contentTypeSave.Alias))) .... }

同様の警告：


フォーマットエラー

PVS-Studioアナライザーの警告 ： V3025の形式が正しくありません 。 「フォーマット」関数を呼び出すときに、異なる数のフォーマット項目が予想されます。 使用されていない書式項目：{1}。 使用されない引数：1番目。 HtmlHelperRenderExtensions.cs 938

 public static IHtmlString EnableCanvasDesigner(....) { .... string noPreviewLinks = @"<link href=""{1}"" type= ""text/css"" rel=""stylesheet" " data-title=""canvasdesignerCss"" />"; .... if (....) result = string.Format(noPreviewLinks, cssPath) + // <= Environment.NewLine; .... }

noPreviewLinks形式文字列には、string.Formatメソッドの最初の引数cssPathの修飾子 '{0}'が含まれていません。 このコードを実行した結果、例外がスローされます。 この設計の修正バージョンの形式は次のとおりです。

 public static IHtmlString EnableCanvasDesigner(....) { .... string noPreviewLinks = @"<link href=""{0}"" type= ""text/css"" rel=""stylesheet" " data-title=""canvasdesignerCss"" />"; .... if (....) result = string.Format(noPreviewLinks, cssPath) + Environment.NewLine; .... }

同様の警告：


nullの等価性をタイミングよくチェックします。 また

PVS-Studioアナライザーの警告 ： V3095 nullに対して検証される前に「データセット」オブジェクトが使用されました。 行を確認してください：48、49。ImageCropperBaseExtensions.cs 48

 internal static ImageCropData GetCrop(....) { var imageCropDatas = dataset.ToArray(); // <= if (dataset == null || imageCropDatas.Any() == false) return null; .... }

1つの条件内でnullの タイミングの悪いチェックが見つかったV3027診断とは異なり、ここでは別のステートメントでnull参照にアクセスしようとしています。 データセット変数は最初に配列に変換され、その後のみnullがチェックされます 。 この設計の修正バージョンの形式は次のとおりです。

 internal static ImageCropData GetCrop(....) { var imageCropDatas = dataset?.ToArray(); if (imageCropDatas == null || !imageCropDatas.Any()) return null; .... }

同様の警告：


論理エラー

PVS-Studioアナライザーの警告 ： V3022式 'name！= "Min" || name！= "Max" 'は常に真です。 ここでは、おそらく「&&」演算子を使用する必要があります。 DynamicPublishedContentList.cs 415

 private object Aggregate(....) { .... if (name != "Min" || name != "Max") // <= { throw new ArgumentException( "Can only use aggregate min or max methods on properties which are datetime"); } .... }

例外のメッセージからわかるように、変数名には「Min」 または 「Max」のいずれかの値のみを使用できます。 同時に、この例外をスローする条件は、変数名 「Min」 と 「Max」の同時不等式である必要があります。 また、指定されたコードフラグメントでは、 nameの値に対して例外がスローされます。 この設計の修正バージョンの形式は次のとおりです。

 private object Aggregate(....) { .... if (name != "Min" && name != "Max") { throw new ArgumentException( "Can only use aggregate min or max methods on properties which are datetime"); } .... }

Umbracoコードでは、32個のより類似した潜在的に安全でない構成体が見つかりました（ただし、それらは単なる余分なチェックであることが判明する場合があります）。 それらのいくつかをあげます。


奇妙なループ状態

PVS-Studioアナライザーの警告 ： V3022式 '！Stop'は常にtrueです。 template.cs 229

 public Control parseStringBuilder(....) { .... bool stop = false; .... while (!stop) // <= { .... } .... }

V3022診断によって検出された別の不審なデザイン。 停止変数は、 whileブロック内では使用されません。 このブロックには、140行程度のかなり大量のコードが含まれているため、ここでは説明しません。 停止変数の検索結果は次のとおりです。


ループは、例外処理ブロックと同様にbreakを含むため 、おそらく無限ではありません。 ただし、ループは非常に奇妙に見え、潜在的なエラーが含まれている場合があります。

無限再帰

PVS-Studioアナライザーの警告 ： V3110 「レンダリング」メソッド内で無限再帰が発生する可能性があります。 MenuSplitButton.cs 30

 protected override void Render(System.Web.UI.HtmlTextWriter writer) { writer.Write("</div>"); base.Render(writer); this.Render(writer); // <= writer.Write("<div class='btn-group>"); }

どうやら、指定されたコードフラグメントには、無限再帰の作成に関連するエラーが含まれています。 基本クラスのRenderメソッドを呼び出した後、オーバーロードされたRenderメソッドは「類推によって」再帰的に呼び出されます。 ほとんどの場合、 this.Renderメソッドには再帰を終了するための条件が含まれている必要があります。 この場合、この設計の正しいバージョンについて明確な結論を出すことは困難です。

おわりに


そのため、Umbracoプロジェクトを再確認すると、PVS-StudioがC＃コードで潜在的に危険な構成要素とエラーのある構成要素の両方の検出に大きな進歩を遂げていることがわかりました。 アナライザーは再びその有効性を証明しました。 もちろん、静的分析の最大の効果は通常の使用で達成されるため、1年に1回の頻度でプロジェクトをチェックしないでください。 これにより、エラーをタイムリーかつ効果的な方法で修正し、アセンブリシステムまたはユーザーに侵入するのを防ぐことができます。

静的解析を使用してください！ そして、誰でもこれを行えるように、アナライザーを自由に使用できる可能性を追加しました。 エラーと完璧なコードとの戦いで頑張ってください！


この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Sergey Khrenov。 umbracoコードの再分析

記事を読んで質問がありますか？
多くの場合、記事には同じ質問が寄せられます。 ここで回答を集めました： PVS-Studioバージョン2015に関する記事の読者からの質問への回答 。 リストをご覧ください。

Source: https://habr.com/ru/post/J317900/

More articles:


All Articles