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

fix(custom_header): not correctly parsing parameters #172

Merged
merged 7 commits into from
Sep 18, 2024

Conversation

psyray
Copy link
Contributor

@psyray psyray commented Aug 28, 2024

Fixes #19
When you set custom header in the scan, value does not been parsed correctly, making httpx crash and not resolving http request.
As httpx is the base of the scan, no other scan will be launched because no URL was found.

I've reworked the run_command and stream_command to be more rock solid.
I've also removed duplicated code

Should fix other problems when parameters contains spaces in their value.

Now custom headers is fully effective and working fine

  • Test command with global headers
  • Test command with task headers
  • Test command with both headers
  • Test command without headers

Summary by Sourcery

Fix the issue with custom header parsing that caused httpx crashes, refactor command execution functions for improved stability, and enhance logging for better debugging.

Bug Fixes:

  • Fix parsing of custom headers to prevent crashes in httpx and ensure proper HTTP request resolution.

Enhancements:

  • Improve the robustness of the run_command and stream_command functions by refactoring and removing duplicated code.
  • Add logging for debugging purposes in various functions, including fetch_url and generate_header_param.

@psyray psyray added the bug Something isn't working label Aug 28, 2024
@psyray psyray self-assigned this Aug 28, 2024
@psyray psyray linked an issue Aug 28, 2024 that may be closed by this pull request
3 tasks
@AnonymousWP AnonymousWP removed their request for review August 29, 2024 12:35
@AnonymousWP
Copy link
Member

Due to ocervell's experience, I think it's better if he (or another developer) from the team reviews this.

@AnonymousWP AnonymousWP requested review from a team and removed request for ocervell August 30, 2024 18:02
web/reNgine/common_func.py Show resolved Hide resolved
@psyray
Copy link
Contributor Author

psyray commented Sep 10, 2024

@sourcery-ai review

Copy link

sourcery-ai bot commented Sep 10, 2024

Reviewer's Guide by Sourcery

This pull request addresses a bug in custom header parsing for scans, improves the robustness of command execution, and enhances logging and error handling. The changes primarily affect the tasks.py and common_func.py files, focusing on refactoring the run_command and stream_command functions, as well as improving the generate_header_param function.

File-Level Changes

Change Details Files
Refactored and improved command execution functions
  • Separated command preparation logic into a new function prepare_command
  • Created a new function create_command_object to handle database operations
  • Improved error handling and logging in command execution
  • Refactored run_command and stream_command for better code organization and readability
web/reNgine/tasks.py
web/reNgine/common_func.py
Enhanced custom header parsing and parameter generation
  • Added debug logging to generate_header_param function
  • Improved the logic for selecting the appropriate header format based on the tool name
web/reNgine/common_func.py
Improved URL handling and error checking in fetch_url function
  • Added checks for empty URL lists
  • Improved logging for URL sources (user-provided vs. database-gathered)
web/reNgine/tasks.py
Added new utility functions for command execution and output processing
  • Created process_line function to handle output line processing
  • Added write_history function to manage command history file operations
  • Implemented execute_command function to standardize subprocess creation
web/reNgine/common_func.py

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @psyray - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

web/reNgine/common_func.py Show resolved Hide resolved
web/reNgine/tasks.py Show resolved Hide resolved
@AnonymousWP AnonymousWP changed the title bug(custom_header): fix not correctly parsed parameters fix(custom_header): not correctly parsing parameters Sep 12, 2024
Copy link
Member

@AnonymousWP AnonymousWP left a comment

Choose a reason for hiding this comment

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

I think this is the only thing I can find. Not sure how much effort I should put into testing every possibility.

web/reNgine/tasks.py Show resolved Hide resolved
- Introduced a clean_quotes function to sanitize input data by removing double quotes.
- Updated form handling in add_engine and update_engine views to use the clean_quotes function for input sanitization.
- Added UTF-8 encoding support to various file operations to ensure proper handling of text files.
- Enhanced parse_custom_header function to validate header values and raise errors for invalid formats.
@AnonymousWP AnonymousWP merged commit 6840691 into release/2.1.0 Sep 18, 2024
4 of 5 checks passed
@AnonymousWP AnonymousWP deleted the 140-fix-global-custom-header branch September 18, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(scan): fetch_url scan fails when a global custom_header is set
2 participants