OdyseeTeam / odysee-api

API server for Odysee
https://odysee.com
MIT License
228 stars 49 forks source link

(#370) Do not log all JSON-RPC parameters #381

Closed lucasgpulcinelli closed 2 years ago

lucasgpulcinelli commented 2 years ago

I was able to solve this issue by creating a function to copy the map of parameters and cut lists inside it to only a fixed size, then this shallow copy is the one that gets logged. I hope this implementation is acceptable.

lucasgpulcinelli commented 2 years ago

What you said makes sense, I added a commit to fix these problems. I took care to make a copy of the list because from what I know slices are not copy-by-value when created, so adding a "skipped" notice at the end would overwrite value[num], possibly creating problems not related to the logging at all.

anbsky commented 2 years ago

possibly creating problems not related to the logging at all.

Good point, would it be possible to write a test to prove that it does not truncate params being passed to the actual JSON-RPC call?

lucasgpulcinelli commented 2 years ago

I am not sure how odysee-api test cases are usually (I mean if a "fake" request parameters is enough, as I did in this last commit). Also, I corrected a small logic error I made in the if case of the function.

anbsky commented 2 years ago

Sorry if I wasn't clear enough — I meant more of an integration test that would ensure that actual parameters passed to Caller.Call as a part of JSON-RPC request never get cut.

But if we're unit-testing the function itself, I would check that it does cut the parameters passed to it.

lucasgpulcinelli commented 2 years ago

I hope I understood the idea of the test correctly now, I tried to get rid of the giant srv.NextResponse however the code would either segfault or throw an EOF error.