Nspv json response buffer fix #530
Conversation
| { | ||
| rpc_result = JSONRPCReplyObj(result, NullUniValue, jreq.id); | ||
| response=rpc_result.write(); | ||
| ptr->json = (char*)malloc(response.size()); |
There was a problem hiding this comment.
I believe it possible that response.size() could be a large number. Should there be checks to assure that the malloc here (and below) did not return nullptr?
There was a problem hiding this comment.
definitely should be checked, thank you
| void NSPV_remoterpc_purge(struct NSPV_remoterpcresp *ptr) | ||
| { | ||
| if ( ptr != 0 ) | ||
| if ( ptr != 0 ) { |
There was a problem hiding this comment.
I do not know much of the NSPV code, so take this as food for thought. Would this be better off in a destructor?
There was a problem hiding this comment.
I totally refactored the nspv code and replaced all such read/write functions to CDataStream object, sent it into research-new branch. So eventually all this code will be replaced
jmjatlanta
left a comment
jmjatlanta
left a comment
There was a problem hiding this comment.
Note that NSPV_rwremoterpcresp is never used to write to the struct, but would be dangerous if it tried to. I suggest either (a) renaming the method and removing the first parameter or (b) asserting if the first parameter is ever a 0.
Also, adding a constructor/destructor to the struct would move the need for the functionality of memset and _purge to a central location.
| { | ||
| char method[64]; | ||
| char json[11000]; | ||
| char *json; |
There was a problem hiding this comment.
Note that in some cases, json != nullptr. It may be better to explicitly initialize it to nullptr here. This will make _purge method less dangerous.
There was a problem hiding this comment.
there is a memset to zero NSPV_remoterpcresp after the var decl
but just added a constructor to auto init to zeros
|
this PR is in the combined #559 PR |
Release v1.0.12
2 participants
No description provided.