Nspv json response buffer fix #530
Conversation
| @@ -688,6 +688,7 @@ int32_t NSPV_remoterpc(struct NSPV_remoterpcresp *ptr,char *json,int n) | |||
| { | |||
| 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
| @@ -441,8 +441,10 @@ int32_t NSPV_rwremoterpcresp(int32_t rwflag,uint8_t *serialized,struct NSPV_remo | |||
|
|
|||
| 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
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.
| @@ -186,7 +186,7 @@ struct NSPV_CCmtxinfo | |||
| struct NSPV_remoterpcresp | |||
| { | |||
| 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
No description provided.