[bug] - Refactor newDiff constructor to avoid double initialization of contentWriter #2742
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 tasks
mcastorina
approved these changes
Apr 25, 2024
Comment on lines
+2385
to
+2386
| mockWriter := newMockContentWriter() | ||
| assert.NotNil(t, mockWriter, "Failed to create mockWriter") |
There was a problem hiding this comment.
Is mockWriter doing anything useful in this test? It seems to be just a test fixture.
There was a problem hiding this comment.
🤦 Thank you for catching this. I updated the test to ensure mockContentWriter implements contentWriter as well as using it as part of the opts for the second test case.
There was a problem hiding this comment.
I'm still not sure I understand how the test catches double initialization. It looks like counter is set to 1 in newMockContentWriter() and never modified, so it'll always be 1 regardless of what newDiff does.
mcastorina
added a commit
that referenced
this pull request
Apr 25, 2024
This wasn't actually testing the fix, which is more difficult to orchestrate than is worth. See: #2742
mcastorina
added a commit
that referenced
this pull request
Apr 25, 2024
This wasn't actually testing the fix, which is more difficult to orchestrate than is worth. See: #2742
haraldh
added a commit
to matter-labs/vault-auth-tee
that referenced
this pull request
May 6, 2024
[](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [trufflesecurity/trufflehog](https://togithub.com/trufflesecurity/trufflehog) | action | minor | `v3.74.0` -> `v3.75.0` | --- ### Release Notes <details> <summary>trufflesecurity/trufflehog (trufflesecurity/trufflehog)</summary> ### [`v3.75.0`](https://togithub.com/trufflesecurity/trufflehog/releases/tag/v3.75.0) [Compare Source](https://togithub.com/trufflesecurity/trufflehog/compare/v3.74.0...v3.75.0) #### What's Changed - \[chore] - update buffer metrics by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2737 - fix(deps): update module github.com/aws/aws-sdk-go to v1.51.28 by [@​renovate](https://togithub.com/renovate) in [trufflesecurity/trufflehog#2741 - chore(deps): update golangci/golangci-lint-action action to v5 by [@​renovate](https://togithub.com/renovate) in [trufflesecurity/trufflehog#2744 - Scan commit metadata by [@​rgmz](https://togithub.com/rgmz) in [trufflesecurity/trufflehog#2713 - Fix SQL Server detector tests by [@​rosecodym](https://togithub.com/rosecodym) in [trufflesecurity/trufflehog#2716 - Revert "Scan commit metadata" by [@​rosecodym](https://togithub.com/rosecodym) in [trufflesecurity/trufflehog#2747 - \[bug] - Refactor newDiff constructor to avoid double initialization of contentWriter by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2742 - \[chore] - update buffered file writer metric by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2740 - \[refactor] - lazy buffer retrieval by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2745 - \[chore] Remove broken test by [@​mcastorina](https://togithub.com/mcastorina) in [trufflesecurity/trufflehog#2748 - \[bug] - fix buffer size metric by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2749 - \[bug] - Fix the metric for buffered file writer writes by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2750 - fix(deps): update module github.com/aws/aws-sdk-go to v1.51.29 by [@​renovate](https://togithub.com/renovate) in [trufflesecurity/trufflehog#2751 - update integration logos by [@​dustin-decker](https://togithub.com/dustin-decker) in [trufflesecurity/trufflehog#2752 - fix(deps): update module github.com/aws/aws-sdk-go to v1.51.30 by [@​renovate](https://togithub.com/renovate) in [trufflesecurity/trufflehog#2756 - \[chore] - add additional binary extension by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2760 - pkg: fix function names in comment by [@​mountcount](https://togithub.com/mountcount) in [trufflesecurity/trufflehog#2761 - \[chore] - ignore pbix and vsdx files by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2762 - fix(deps): update module github.com/aws/aws-sdk-go to v1.51.31 by [@​renovate](https://togithub.com/renovate) in [trufflesecurity/trufflehog#2763 - Scan commit metadata by [@​rgmz](https://togithub.com/rgmz) in [trufflesecurity/trufflehog#2754 - \[bug] - Correctly set metrics for enumerated orgs by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2757 - \[chore ] -Update ignore extensions by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2764 - \[chore] Add some happy path logs to GitLab by [@​mcastorina](https://togithub.com/mcastorina) in [trufflesecurity/trufflehog#2765 - Fix Git source test by [@​rgmz](https://togithub.com/rgmz) in [trufflesecurity/trufflehog#2767 - \[feat] - buffered file reader by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2731 - \[feat] - Add ReadFrom method to BufferedFileWriter by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2759 - fix(deps): update module google.golang.org/protobuf to v1.34.0 by [@​renovate](https://togithub.com/renovate) in [trufflesecurity/trufflehog#2766 - \[bug] - Improve BufferedFileReader Close Behavior by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2768 - fixes calendly api key regex by [@​ankushgoel27](https://togithub.com/ankushgoel27) in [trufflesecurity/trufflehog#2368 - Expose detector-specific false positive logic by [@​rosecodym](https://togithub.com/rosecodym) in [trufflesecurity/trufflehog#2743 - Detector-Fix: Reintroduce Cloudflareglobalapikey by [@​ankushgoel27](https://togithub.com/ankushgoel27) in [trufflesecurity/trufflehog#2101 - Detector-Competition-Fix - fixed the alchemy detector regex by [@​ankushgoel27](https://togithub.com/ankushgoel27) in [trufflesecurity/trufflehog#1821 - fix(deps): update module github.com/aws/aws-sdk-go to v1.51.32 by [@​renovate](https://togithub.com/renovate) in [trufflesecurity/trufflehog#2769 - fix(deps): update module google.golang.org/api to v0.177.0 by [@​renovate](https://togithub.com/renovate) in [trufflesecurity/trufflehog#2770 - \[chore] - update imports by [@​ahrav](https://togithub.com/ahrav) in [trufflesecurity/trufflehog#2772 - adds build version to finished scanning log by [@​zricethezav](https://togithub.com/zricethezav) in [trufflesecurity/trufflehog#2773 - Update rabbitmq.go regex detect amqps protocol by [@​NikhilPanwar](https://togithub.com/NikhilPanwar) in [trufflesecurity/trufflehog#2609 - fix for infinite recursion in Postman var sub by [@​zricethezav](https://togithub.com/zricethezav) in [trufflesecurity/trufflehog#2780 #### New Contributors - [@​mountcount](https://togithub.com/mountcount) made their first contribution in [trufflesecurity/trufflehog#2761 **Full Changelog**: trufflesecurity/trufflehog@v3.74.0...v3.75.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/matter-labs/vault-auth-tee). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMzEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjMzMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
](https://renovatebot.com){kind=link}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This PR addresses an issue in the
newDiffconstructor where thecontentWritercould potentially be double initialized, leading to duplicate resource creation and potential resource leaks.Suggested improvement for bufferwriter package:
While investigating the issue with the newDiff constructor, it became apparent that the current design of the bufferwriter package could be improved to make it more ergonomic and less prone to misuse.
Instead of allocating resources (i.e., the underlying buffer) in the
Newconstructor, we should consider modifying the implementation to allocate resources lazily, only when theWritemethod is called for the first time. This lazy allocation approach would prevent potential resource leaks and make it easier to use the package correctly.Checklist:
make test-community)?make lintthis requires golangci-lint)?