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

Add SliceContainsString common util #3395

Merged
merged 2 commits into from
Oct 10, 2024
Merged

Conversation

bill-rich
Copy link
Collaborator

No description provided.

@bill-rich bill-rich requested a review from a team as a code owner October 10, 2024 19:35
Copy link

@sysread sysread left a comment

Choose a reason for hiding this comment

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

Personally, I would include the matched index in the return value, but that's a nitpick for sure.

targetString = strings.ToLower(origTargetString)
}
for _, stringFromSlice := range stringSlice {
if ignoreCase {
Copy link
Collaborator

@ahrav ahrav Oct 10, 2024

Choose a reason for hiding this comment

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

suggestion: If ignoreCase is true, we can avoid the overhead of calling strings.ToLower on each slice item by performing the conversion once for origTargetString.

Ex:

if ignoreCase {
		targetString := strings.ToLower(origTargetString)
		for _, stringFromSlice := range stringSlice {
			if targetString == strings.ToLower(stringFromSlice) {
				return true, origTargetString
			}
		}
	} else {
		for _, stringFromSlice := range stringSlice {
			if origTargetString == stringFromSlice {
				return true, origTargetString
			}
		}
	}
	return false, ""

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It currently only does the ToLower conversion if ignoreCase is true. The original way does the check on every iteration, uses the same loop.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh right duh!

@@ -81,3 +81,20 @@ func GetAccountNumFromAWSID(AWSID string) (string, error) {
accountNum := (z & mask) >> 7
return fmt.Sprintf("%012d", accountNum), nil
}

// SliceContainsString searches a slice to determine if it contains a specified string.
func SliceContainsString(origTargetString string, stringSlice []string, ignoreCase bool) (bool, string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: How large are you anticipating stringSlice being?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I imagine most cases won't be huge, but didn't really have a limit in mind.

@bill-rich
Copy link
Collaborator Author

Personally, I would include the matched index in the return value, but that's a nitpick for sure.

Good call. Added

@bill-rich bill-rich merged commit 5280c38 into main Oct 10, 2024
13 checks passed
@bill-rich bill-rich deleted the add_string_in_slice_common_util branch October 10, 2024 20:23
// SliceContainsString searches a slice to determine if it contains a specified string.
// Returns the index of the first match in the slice.
func SliceContainsString(origTargetString string, stringSlice []string, ignoreCase bool) (bool, string, int) {
targetString := origTargetString
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: How come you created targetString and stringFromSlice vars as opposed to updating the existing values? Was it solely for clarity?

if ignoreCase {
    origTargetString = strings.ToLower(origTargetString)
}

similarly for:

if ignoreCase {
    origStringFromSlice = strings.ToLower(origStringFromSlice)
}

abmussani added a commit to abmussani/trufflehog that referenced this pull request Oct 14, 2024
* main: (127 commits)
  Update SaladCloud description (trufflesecurity#3399)
  fix tests (trufflesecurity#3400)
  [chore] Update custom detector default description (trufflesecurity#3398)
  add description to salad (trufflesecurity#3397)
  Add detector for SaladCloud API Keys (trufflesecurity#3273)
  fix(deps): update module github.com/xanzy/go-gitlab to v0.111.0 (trufflesecurity#3393)
  Add SliceContainsString common util (trufflesecurity#3395)
  fix: pr template link to golangci-lint (trufflesecurity#3392)
  fix(deps): update golang.org/x/exp digest to f66d83c (trufflesecurity#3389)
  Separate detector tests into unit/integration (trufflesecurity#3274)
  Manually upgrade github dep (trufflesecurity#3387)
  Updated Fastly Personal Token Detector (trufflesecurity#3386)
  fix(deps): update module google.golang.org/api to v0.200.0 (trufflesecurity#3391)
  [Fix] Snowflake privatelink Support (trufflesecurity#3286)
  Enhanced the easyinsight detector (trufflesecurity#3384)
  Log skipped files on debug level (trufflesecurity#3383)
  build: update retracted bluemonday ver (trufflesecurity#3369)
  Fix git binary handling and add a smoke test (trufflesecurity#3379)
  fix(deps): update module google.golang.org/protobuf to v1.35.1 (trufflesecurity#3382)
  Added Cisco Meraki API Key detector (trufflesecurity#3367)
  ...

# Conflicts:
#	pkg/engine/defaults.go
#	pkg/pb/detectorspb/detectors.pb.go
#	proto/detectors.proto
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants