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.

Sign up for GitHub

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

Jump to bottom

Improved error message on parameters decode and "" case. #222

Merged
merged 4 commits into from
Jun 22, 2023
Merged

Improved error message on parameters decode and "" case. #222

merged 4 commits into from
Jun 22, 2023

Conversation

hulto
Copy link
Collaborator

@hulto hulto commented Jun 19, 2023 •
edited

What type of PR is this?

/kind bug
/kind cleanup

What this PR does / why we need it:

This adds support for the case where parameters = "" this is the value when parameters is not specified by the c2 server.

Also adds improved error handling around parameter decoding.
image

This PR also adds error handling around other uses of ? in eldritch_run to better show why tomes fail and hopefully speed up trouble shooting in the future.

Which issue(s) this PR fixes:

Fixes #221

@hulto hulto added bug Something isn't working cleanup Code cleanup and tech debt removal imix labels Jun 19, 2023
@hulto hulto linked an issue Jun 19, 2023 that may be closed by this pull request
imix doesn't handle empty parameters field. #221
Closed
@codecov
Copy link

codecov bot commented Jun 19, 2023 •
edited

Codecov Report

Merging #222 (1939a75) into main (88d9040) will decrease coverage by 1.93%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##             main     #222      +/-   ##
==========================================
- Coverage   72.33%   70.40%   -1.93%     
==========================================
  Files          82       82              
  Lines        4667     4633      -34     
==========================================
- Hits         3376     3262     -114     
- Misses       1206     1286      +80     
  Partials       85       85
Impacted Files Coverage Δ
implants/eldritch/src/lib.rs 92.22% <75.00%> (-1.36%) ⬇️

... and 9 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Cictrone
Cictrone approved these changes Jun 22, 2023
View reviewed changes
Copy link
Collaborator

@Cictrone Cictrone left a comment

Choose a reason for hiding this comment

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

in general looking gucci and im a fan of any amount of removal of "?" in favor of error handling. Just had one question but other than that gucci.

implants/eldritch/src/lib.rs Show resolved Hide resolved
@hulto hulto merged commit 18b911e into main Jun 22, 2023
7 of 8 checks passed
@hulto hulto deleted the 221-imix-doesnt-handle-empty-parameters-field branch June 22, 2023 19:55
KCarretto pushed a commit that referenced this pull request Feb 1, 2024
@hulto
bb6712b 
 
Improved error message on parameters decode and "" case. (#222)

* Improved error message on decode and "" case.

* Add error handling to other eldritch_run calls.

* Fix test and update imix -> eldritch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Cictrone Cictrone Cictrone approved these changes

Assignees
No one assigned
Labels
bug Something isn't working cleanup Code cleanup and tech debt removal imix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

imix doesn't handle empty parameters field.
2 participants
@hulto @Cictrone