-
Notifications
You must be signed in to change notification settings - Fork 54
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
Fix boolean parsing in OpenVPNPlugin #479
Conversation
@Repsay 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.
|
Hi @Repsay, thanks for the bug fix! Looking a bit further into the issue I think this issue is better fixed in the To have a general fix for these kind of options we could replace the line:
in
Feel free to update your PR. Otherwise we can open a bug issue and will get to fixing it in due time. |
Hi @pyrco, thank you for the suggestion to refine the issue in the To address this, I've already updated the pull request. In addition to your suggestion, I've incorporated a dynamic way to identify boolean fields with the line Here's the revised section of the code: if any(key == boolean_field.name for boolean_field in boolean_fields):
value = value[0] if value else True
else:
value = value[0] if value else ""
# This removes all text after the first comment
value = CONFIG_COMMENT_SPLIT_REGEX.split(value, 1)[0].strip() This update should offer a more robust and comprehensive solution to handle the OpenVPN configuration options correctly. Please review the changes in the updated PR, and let me know if you have any further suggestions or if additional modifications are required. |
@DissectBot agree |
@pyrco, I've updated the code incorporating your recommendations. Please take a look and let me know if there's anything else you'd like adjusted. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #479 +/- ##
==========================================
- Coverage 73.96% 73.96% -0.01%
==========================================
Files 260 260
Lines 21034 21038 +4
==========================================
+ Hits 15558 15560 +2
- Misses 5476 5478 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Resolved an issue in the OpenVPNPlugin where the 'client-to-client' configuration option was incorrectly processed. Previously, the code attempted to use the value directly from the configuration, which led to errors when this value was an empty string. This fix ensures that an empty string for 'client-to-client' defaults to False, aligning with the expected boolean behavior of this configuration parameter. This change enhances the robustness of the OpenVPN configuration parsing, preventing type conversion errors and ensuring correct interpretation of the 'client-to-client' setting.
Made the change general for all boolean values
Made changes based on suggestions.
e97085e
to
05f05b0
Compare
Resolved an issue in the OpenVPNPlugin where the 'client-to-client' configuration option was incorrectly processed. Previously, the code attempted to use the value directly from the configuration, which led to errors when this value was an empty string. This fix ensures that an empty string for 'client-to-client' defaults to False, aligning with the expected boolean behavior of this configuration parameter.