Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single match expression should run multiple regex types #553

Merged
merged 6 commits into from
Sep 9, 2021

Conversation

eddynaka
Copy link
Collaborator

@eddynaka eddynaka commented Sep 8, 2021

Changes

Please provide a brief description of the changes here.

For significant contributions please make sure you have completed the following items:

  • ReleaseHistory.md updated for non-trivial changes
  • Added unit tests

@@ -454,24 +454,32 @@ private static void MergeDictionary(IList<Dictionary<string, FlexMatch>> mergeFr

private void RunMatchExpression(FlexMatch binary64DecodedMatch, AnalyzeContext context, MatchExpression matchExpression)
{
bool isMalformed = true;
Copy link
Collaborator Author

@eddynaka eddynaka Sep 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isMalformed

this change will enable a rule to have:

  • ContentsRegex
  • IntrafileRegexes
  • SinglelineRegexes

the idea is to re-use the match expression instead of duplicating it. #ByDesign

@@ -314,9 +314,9 @@ private Assembly CurrentDomain_AssemblyResolve(object sender, ResolveEventArgs a
// We will only attempt to resolve an assembly a single time
// to avoid re-entrance in cases where our logic below fails
string assemblyName = args.Name.Split(',')[0];
if (this._resolvedNames.ContainsKey(assemblyName))
if (this._resolvedNames.TryGetValue(assemblyName, out resolved))
Copy link
Collaborator Author

@eddynaka eddynaka Sep 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TryGetValue

performance change. #ByDesign

@eddynaka eddynaka changed the title Single rule can run multiple regex type Single match expression can run multiple regex types Sep 8, 2021
@eddynaka eddynaka changed the title Single match expression can run multiple regex types Single match expression should run multiple regex types Sep 8, 2021
@@ -22,6 +22,8 @@
`SEC101/036.MySqlCredential`, `SEC101/037.SqlCredentials`,
`SEC101/038.PostgreSqlCredentials` won't accept spaces in `id` and `secret`.
[#550](https://github.com/microsoft/sarif-pattern-matcher/pull/550)
- PRF: Single match expression can run multiple regex types.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PRF

SDK?


if (matchExpression.IntrafileRegexes?.Count > 0 ||
matchExpression.SingleLineRegexes?.Count > 0)
else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else

use else if { here

@@ -314,9 +314,9 @@ private Assembly CurrentDomain_AssemblyResolve(object sender, ResolveEventArgs a
// We will only attempt to resolve an assembly a single time
// to avoid re-entrance in cases where our logic below fails
string assemblyName = args.Name.Split(',')[0];
if (this._resolvedNames.ContainsKey(assemblyName))
if (this._resolvedNames.TryGetValue(assemblyName, out resolved))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this

'this.' is not the standard for this file.

Copy link
Member

@michaelcfanning michaelcfanning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@eddynaka eddynaka merged commit baa9518 into main Sep 9, 2021
@eddynaka eddynaka deleted the users/ednakamu/enabling-multiple-regex-types branch September 9, 2021 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants