feat: re-implement go jsonrpc detector with net/rpc.Request#335
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #335 +/- ##
==========================================
- Coverage 69.82% 69.76% -0.06%
==========================================
Files 213 213
Lines 22230 22213 -17
==========================================
- Hits 15522 15497 -25
- Misses 5908 5915 +7
- Partials 800 801 +1
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:
|
mariomac
left a comment
mariomac
left a comment
There was a problem hiding this comment.
Good stuff!! When you remove the draft status I will approve and merge it unless @rafaelroquetto has extra comments.
titaneric
force-pushed
the
feat/go-jsonrpc-v2
branch
from
f179ceb to
dd345c3
Compare
rafaelroquetto
left a comment
rafaelroquetto
left a comment
There was a problem hiding this comment.
LGTM, thank you so much for doing this. All I have is a bunch of nits regarding const-correctness and clarity, but the code looks good.
And I will forward your thanks to @grcevski as he's the one who actually came up with this insight in first place :)
grcevski
left a comment
grcevski
left a comment
There was a problem hiding this comment.
Excellent work! I just left a small comment about a name of a function.
| return 1; | ||
| } | ||
|
|
||
| static __always_inline u64 peak_go_str_len(const char *name, const void *base_ptr, u8 offset) { |
There was a problem hiding this comment.
Small nit, just a spelling mistake, it should be peek_go_str_len :).
5 participants
As @rafaelroquetto suggest here, I change the uprobe from
http.bodytojsonrpcReadRequestHeader, and remove the old implememtation completely.This new implementation expects more robust and stable detection, and possibly resolve flaky test reported inhttps://github.com//issues/273
Special thanks for @rafaelroquetto!