Add _field_types attribute to GroupedRecord#171
Conversation
|
Thanks! Could you also add a regression test that tests the old broken situation? |
|
@B0TAxy thank you for your contribution! As this is your first code contribution, please read the following Contributor License Agreement (CLA). If you agree with the CLA, please reply with the following information:
Contributor License Agreement
Contribution License AgreementThis Contribution License Agreement ("Agreement") governs your Contribution(s) (as defined below) and conveys certain license rights to Fox-IT B.V. ("Fox-IT") for your Contribution(s) to Fox-IT"s open source Dissect project. This Agreement covers any and all Contributions that you ("You" or "Your"), now or in the future, Submit (as defined below) to this project. This Agreement is between Fox-IT B.V. and You and takes effect when you click an “I Accept” button, check box presented with these terms, otherwise accept these terms or, if earlier, when You Submit a Contribution.
|
|
@DissectBot agree |
Hi, |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in record_modifier.py where a GroupedRecord instance would crash due to the missing _field_types attribute. The changes include adding an expected field type mapping in the tests and initializing _field_types in the base GroupedRecord constructor.
- Updated tests in tests/test_record.py to verify the _field_types attribute.
- Modified flow/record/base.py to initialize the _field_types attribute based on the RecordDescriptor.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_record.py | Added new assertions to check the _field_types attribute and expected field mappings; minor spelling errors in variable/comments need correction. |
| flow/record/base.py | Initializes _field_types to maintain compatibility with RecordDescriptor. |
yunzheng
left a comment
There was a problem hiding this comment.
Looks good, thanks for the pointer where it goes wrong. It seems that dissect.target is using a private attribute to get the fields.
Can you apply the above suggestions by copilot? PR looks good otherwise for merge.
I've addressed the suggestions provided by Copilot. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #171 +/- ##
=======================================
Coverage 82.84% 82.84%
=======================================
Files 34 34
Lines 3573 3574 +1
=======================================
+ Hits 2960 2961 +1
Misses 613 613
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
When using
--hash,--resolve, or both, there's a script called record_modifier.py that tries to find the fields and then performs the requested action, adding a new field afterward. However, since theGroupedRecordclass doesn’t have theself._field_typesattribute, it just crashes every time aGroupedRecordis involved. This code fixes that issue.Fixes #168