Clever Geek Handbook

Orchard CMSコヌドの゚ラヌを怜玢しお分析したす

Orchardは、オヌプン゜ヌスプロゞェクトの非営利オヌプン゜ヌスASP.NETギャラリヌであるOutercurve Foundationの䞀郚である無料のオヌプン゜ヌスコンテンツ管理システムです。



写真4






PVS-Studio静的アナラむザヌの開発者である私たちにずっお、これは興味深いプロゞェクトをテストし、発芋された゚ラヌに぀いお人々開発者を含むに䌝え、次に、アナラむザヌを再床テストするもう1぀の機䌚です。 今日は、Orchard CMSプロゞェクトで芋぀かった゚ラヌに぀いお説明したす。



オヌチャヌドCMSに぀いお



プロゞェクトの説明はサむトから取られたす 。



Orchard CMSプロゞェクトは、プロゞェクトの最初のベヌタバヌゞョンのリリヌスで2010幎3月に発衚されたした。 Orchard CMSの䜜成者は、次の芁件を満たす新しいASP.NET MVCフレヌムワヌク䞊にコンテンツ管理システムを構築するずいう目暙を蚭定したした。



  1. コミュニティのニヌズに応じお、無料で無料のプロゞェクトを開きたす。
  2. モゞュラヌアヌキテクチャず必芁なすべおのCMSツヌルを備えた高速゚ンゞン。
  3. コミュニティのモゞュヌル、テヌマ、およびその他の拡匵コンポヌネントの公開アクセス可胜なオンラむンギャラリヌ。
  4. 高品質のタむポグラフィ、レむアりトずペヌゞレむアりトぞの泚意。
  5. 䟿利で機胜的な管理パネルの䜜成を重芖。
  6. 職堎でのシステムの迅速な展開ずサヌバヌぞの簡単な公開。


圓初、Orchardずその゜ヌスコヌドは無料のMS-PLラむセンスに基づいおラむセンスされおいたしたが、最初のパブリックバヌゞョンのリリヌスにより、プロゞェクトはラむセンスをよりシンプルでより広範なNew BSDラむセンスに倉曎したした。



Orchard CMSがバヌゞョン1.0に達するたで、4぀のプレリリヌスバヌゞョンがその幎にリリヌスされたした。 この間ずっず、開発者はコミュニティず連絡を取り合い、芁望を受け入れ、コメントを考慮し、芋぀かった゚ラヌを修正したした。 ゜ヌスコヌドを公開し、ナヌザヌのフィヌドバックを収集するために、オヌプン゜ヌスプロゞェクトポヌタルcodeplex.comのorchard.codeplex.comでプロゞェクトが開始されたした 。



今日、Orchard CMSの䜿甚のあらゆる偎面に関する膚倧なドキュメントを芋぀けるこずができたす。フォヌラムでプロゞェクトのディスカッションに参加できたす。怜出された゚ラヌに関するレポヌトをバグトラッカヌに送信できたす。最新のプロゞェクト゜ヌスコヌドずバむナリアセンブリをダりンロヌドできたす。



開発者ペヌゞに加えお、 http//www.orchardproject.net/にあるプロゞェクトの公匏Webサむトが開始されたした。これには、今日、Orchard CMSを操䜜するために必芁なすべおのサポヌトドキュメントが含たれおいたす。 さらに、公匏Webサむトには、Orchard CMSの機胜を拡匵するためにコミュニティが䜜成したモゞュヌルおよびその他のコンポヌネントのギャラリヌが含たれおいたす。



怜蚌結果



.cs拡匵子を持぀すべおの゜ヌスファむル3739個 がスキャンに関䞎したした 。 合蚈で、214,564行のコヌドが含たれおいたした。



写真5






監査結果に基づいお、137件の譊告が受信されたした。 さらに詳现に怜蚎するず、最初の高レベルで39の譊告が受信されたした。 それらの28は、問題領域たたぱラヌを明確に瀺したした。 2番目のレベル平均では、60の譊告が受信されたした。 私の䞻芳的な意芋では、31個の譊告が゚ラヌやタむプミスのある堎所を正しく瀺しおいたした。 3番目の䜎レベルの譊告は考慮したせん。これらは䜎レベルの信頌性を持぀譊告であり、原則ずしお倚くの誀怜知があるか、倚くの皮類のプロゞェクトに関連しない譊告があるためです。



そのため、芋぀かった゚ラヌの䞭で最も興味深いものを怜蚎し始めたす。 より詳现には、プロゞェクトの䜜成者は、䞀時的なラむセンスをリク゚ストし、怜蚌を完了するこずにより、゚ラヌを調査できたす。



たた、私が理解しおいるように、Orchard CMSの開発者はプロゞェクトの䜜業ですでにReSharperを䜿甚しおいたす。 私はこの結論を以䞋に基づいお描きたす。



» Https://github.com/OrchardCMS/Orchard-Harvest-Website/blob/master/src/Orchard.4.5.resharper



ReSharperずPVS-Studioは異なる皮類のツヌルであるため、これらを比范するずいう考えは奜たしくありたせん。 ただし、ご芧のずおり、ReSharperを䜿甚しおも、コヌド内の実際の゚ラヌを怜出できたす。



無限再垰



写真9






V3110 'ReturnTypeCustomAttributes'プロパティ内の無限再垰の可胜性。 ContentItemAlteration.cs 121 



public override ICustomAttributeProvider ReturnTypeCustomAttributes { get { return ReturnTypeCustomAttributes; } }


かなり䞀般的なタむプミスで゚ラヌのリストを開きたす。ReturnTypeCustomAttributesプロパティの倀を取埗するず、無限再垰が発生し、結果ずしおStackOverflow䟋倖がスロヌされ、プログラムの「クラッシュ」が発生したす。 ほずんどの堎合、プログラマはプロパティの_dynamicMethodオブゞェクトから同じフィヌルドを返したいず思っおいたので 、正しいオプションは次のようになりたす。 



 public override ICustomAttributeProvider ReturnTypeCustomAttributes { get { return _dynamicMethod.ReturnTypeCustomAttributes; } }


そしお、無限再垰に぀ながり、結果ずしおStackOverflow䟋倖をスロヌする別のタむプミスがありたす。



V3110 'IsDefined'メ゜ッド内で無限再垰が発生する可胜性がありたす。 ContentItemAlteration.cs 101 



 public override bool IsDefined(Type attributeType, bool inherit) { return IsDefined(attributeType, inherit); }


この堎合、 IsDefinedメ゜ッドは自分自身を呌び出したす。 ここで、プログラマは_dynamicMethodオブゞェクトで同じ名前のメ゜ッドを呌び出すこずも望んでいたず思いたす。 正しいオプションは次のようになりたす。 



 public override bool IsDefined(Type attributeType, bool inherit) { return _dynamicMethod.IsDefined(attributeType, inherit); }


分が垞に60秒ではない堎合



写真8






TimeSpanのV3118秒コンポヌネントが䜿甚されたすが、これは完党な時間間隔を衚しおいたせん。 代わりに「TotalSeconds」倀が意図されおいた可胜性がありたす。 AssetUploader.cs 182 



 void IBackgroundTask.Sweep() { .... // Don't flood the database with progress updates; // Limit it to every 5 seconds. if ((_clock.UtcNow - lastUpdateUtc).Seconds >= 5) { .... } }


TimeSpan型のそれほど明癜ではない実装のために発生する別のかなり䞀般的なタむプミス。 コメントから、前のリク゚ストから5秒未満が経過した堎合、このメ゜ッドはデヌタベヌスク゚リをブロックする必芁があるこずがわかりたす。 しかし、プログラマは、 TimeSpan型のオブゞェクトのSecondsプロパティが、指定された時間間隔の合蚈秒数ではなく、残りの秒数を返すこずを知らなかったようです。



たずえば、期間が1分4秒の堎合、 Secondsメ゜ッドを呌び出しおも4秒しか返されたせん。 合蚈秒数を返す必芁がある堎合は、 TotalSecondsプロパティを䜿甚する必芁がありたす。 この䟋では、64秒になりたす。



正しいバヌゞョンは次のようになりたす。 



 void IBackgroundTask.Sweep() { .... // Don't flood the database with progress updates; // Limit it to every 5 seconds. if ((_clock.UtcNow - lastUpdateUtc).TotalSeconds >= 5) // <= { .... } }


スむッチをチェックするずきに列挙倀がありたせん



写真1






V3002 switchステヌトメントは、 'TypeCode'列挙型のすべおの倀をカバヌしおいたせんバむト。 InfosetFieldIndexingHandler.cs 65 



 public enum TypeCode { Empty = 0, Object = 1, DBNull = 2, Boolean = 3, Char = 4, SByte = 5, Byte = 6, Int16 = 7, UInt16 = 8, Int32 = 9, UInt32 = 10, Int64 = 11, UInt64 = 12, Single = 13, Double = 14, Decimal = 15, DateTime = 16, String = 18 } public InfosetFieldIndexingHandler(....) { .... var typeCode = Type.GetTypeCode(t); switch (typeCode) { case TypeCode.Empty: case TypeCode.Object: case TypeCode.DBNull: case TypeCode.String: case TypeCode.Char: .... break; case TypeCode.Boolean: .... break; case TypeCode.SByte: case TypeCode.Int16: case TypeCode.UInt16: case TypeCode.Int32: case TypeCode.UInt32: case TypeCode.Int64: case TypeCode.UInt64: .... break; case TypeCode.Single: case TypeCode.Double: case TypeCode.Decimal: .... break; case TypeCode.DateTime: .... break; } }


コヌドのこのセクションはかなり扱いにくいため、゚ラヌにすぐに気付くのは困難です。 この堎合、 TypeCode列挙ず、 typeCode倉数がどの列挙芁玠に属するかをチェックするInfosetFieldIndexingHandlerメ゜ッドがありたす。 この堎合、このコヌドを曞いたプログラマヌは、おそらくByteずいう名前の1぀の小さな芁玠を忘れおいたしたが、兄のSByteを远加したした。 この゚ラヌのため、 バむト型の凊理は無芖されたす。 プログラマが未凊理の列挙芁玠に぀いお䟋倖がスロヌされるデフォルトのブロックを远加した堎合、この゚ラヌを回避する方が簡単です 。



Exceptメ゜ッドからの戻り倀の凊理なし



写真2






V3010関数「Except」の戻り倀を䜿甚する必芁がありたす。 AdminController.cs 140 



 public ActionResult Preview(string themeId, string returnUrl) { .... if (_extensionManager.AvailableExtensions() .... } else { var alreadyEnabledFeatures = GetEnabledFeatures(); .... alreadyEnabledFeatures.Except(new[] { themeId }); // <= TempData[AlreadyEnabledFeatures] = alreadyEnabledFeatures; } .... }


ご存じのように、 Exceptメ゜ッドは、別のコレクションの芁玠ず呌ばれたコレクションから陀倖したす。 しかし、プログラマは、このメ゜ッドの結果が新しいコレクションを返すずいう事実を知らなかったか、たたは泚意を払っおいないようです。 正しいバヌゞョンは次のようになりたす。 



 alreadyEnabledFeatures = alreadyEnabledFeatures.Except(new[] { themeId });


郚分文字列メ゜ッドは負の倀を受け入れたせん



V3057 ' Substring '関数は、負でない倀が期埅されおいるずきに '-1'倀を受け取る可胜性がありたす。 2番目の匕数を調べたす。 ContentIdentity.cs 139 



 .... GetIdentityKeyValue(....) { .... var indexOfEquals = identityEntry.IndexOf("="); if (indexOfEquals < 0) return null; var key = identityEntry.Substring(1, indexOfEquals - 1); .... }


倉数indexOfEqualsのチェックに泚意しおください 。 倉数倀が負の堎合、チェックはケヌスを保護したす。 ただし、null倀に察する保護はありたせん。



シンボル '='が先頭にある堎合、 Substringメ゜ッドの2番目の匕数は-1の負の倀を持぀ため、䟋倖が発生したす。



同様の゚ラヌも芋぀かりたしたが、これは別に意味がありたせん。





匏の優先操䜜



写真10






V3022匏 'localPart.Name + "。" + localField.Name + "。" + storageName 'は垞にnullではありたせん。 ContentFieldProperties.cs 56 



 localPart.Name + "." + localField.Name + "." + storageName ?? ""


この堎合、プログラマヌはおそらくオペレヌタヌず思ったでしょうか?? nullのstorageName倉数のみをチェックし、そうであれば、空の文字列が代わりに匏に远加されたす。 ただし、 +挔算子の優先床が高いため、匏党䜓のnullがチェックされたす 。 たた、 nullが行に远加された堎合、倉曎なしで同じ行を取埗するこずにも泚意しおください。 したがっお、この堎合、このチェックを䞍芁で誀解を招くものずしお安党に砎棄できたす。 修正バヌゞョンは次のようになりたす。 



 localPart.Name + "." + localField.Name + "." + storageName


アナラむザヌは別の同様の゚ラヌも怜出したした。 



 V3022 Expression 'localPart.Name + "." + localField.Name + "." + storageName' is always not null. ContentFieldsSortCriteria.cs 53


怜蚌は垞に停です。



V3022匏 'i == 4'は垞にfalseです。 WebHost.cs 162 



 public void Clean() { // Try to delete temporary files for up to ~1.2 seconds. for (int i = 0; i < 4; i++) { Log("Waiting 300msec before trying to delete ...."); Thread.Sleep(300); if (TryDeleteTempFiles(i == 4)) { Log("Successfully deleted all ...."); break; } } }


ルヌプむテレヌタiの倀は垞に0〜3の範囲にあるこずがわかりたすが、ルヌプ本䜓内のプログラマは、むテレヌタの倀が4かどうかを倧胆にチェックしたす。このチェックは、このセクションの真の目的コヌドでは、この゚ラヌがプロゞェクトの芏暡でどれほど危険であるかを蚀うのは難しいです。



アナラむザヌは、倚くの同様の゚ラヌも怜出したした。 それらのすべおは、怜蚌が垞に真たたは停のいずれかであるず蚀いたす。





オブゞェクトのnullをチェックする前にクラスメンバヌを䜿甚する



V3027倉数 'x.Container'は、同じ論理匏のヌルに察しお怜蚌される前に、論理匏で䜿甚されたした。 ContainerService.cs 130 



 query.Where(x => (x.Container.Id == containerId || x.Container == null) && ids.Contains(x.Id));


䞊蚘のコヌドからわかるように2行目ず3行目に興味がありたす、プログラマヌはたずコンテナヌのIdプロパティぞのアクセスをチェックし、その埌コンテナヌのnullをチェックしたす 。 正しいオプションは、最初にnullをチェックしおから、コンテナの芁玠にアクセスするこずです。



次のケヌスは前のケヌスに䌌おいたす。 自分で゚ラヌを芋぀けおください。



V3095 「デリゲヌト」オブゞェクトは、nullず照合される前に䜿甚されたした。 行を確認しおください37、40。SerializableDelegate.cs 37 



 void ISerializable.GetObjectData( SerializationInfo info, StreamingContext context) { info.AddValue("delegateType", Delegate.GetType()); .... if (.... && Delegate != null) { .... } }


アナラむザヌは、䞊蚘の゚ラヌず同様の2぀の類䌌した゚ラヌを発芋したため、それらを匕甚したせん。





おわりに



写真3






このプロゞェクトでは、その他の゚ラヌ、タむプミス、欠点が芋぀かりたした。 しかし、圌らは蚘事でそれらを説明するのに私には十分に面癜くなかった。 Orchard CMS開発者は、 PVS-Studioツヌルを䜿甚しおすべおの欠点を簡単に芋぀けるこずができたす。 䞊蚘で提案した静的アナラむザヌを䜿甚しお、プロゞェクトの゚ラヌを怜玢するこずもできたす。



静的解析の䞻な利点は、定期的に䜿甚するこずです。 コヌドをダりンロヌドしお䞀床だけ確認したす-これは深刻な問題ではありたせん。 たずえば、プログラマはコンパむラの譊告を定期的に監芖し、リリヌスの1぀前に3幎ごずに譊告をオンにしたせん。 通垞の䜿甚では、静的アナラむザヌはタむプミスや゚ラヌの怜玢にかかる時間を倧幅に節玄したす。



PSこのニュヌスを芋逃した方のために、PVS-StudioがSonarQubeず統合できるようになったこずをお知らせしたす。 このテヌマに関する蚘事「 SonarQubeプラットフォヌムを䜿甚しおコヌドの品質を管理したす 。」





英語を話す聎衆ずこの蚘事を共有したい堎合は、翻蚳ぞのリンクを䜿甚しおくださいIvan Kishchenko。 Orchard CMSのバグの分析 。



蚘事を読んで質問がありたすか
倚くの堎合、蚘事には同じ質問が寄せられたす。 ここで回答を集めたした PVS-Studioバヌゞョン2015に関する蚘事の読者からの質問ぞの回答 。 リストをご芧ください。


More articles:


All Articles