src: add args validation method#56487
Conversation
|
Review requested:
|
34769a1 to
e0af101
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #56487 +/- ##
==========================================
+ Coverage 88.52% 89.17% +0.65%
==========================================
Files 660 662 +2
Lines 190900 191683 +783
Branches 36628 36890 +262
==========================================
+ Hits 168995 170940 +1945
+ Misses 15090 13612 -1478
- Partials 6815 7131 +316
🚀 New features to boost your workflow:
|
b4c8fd8 to
8bfc0fb
Compare
| CHECK(args[2]->IsUint32()); | ||
| Environment* env = Environment::GetCurrent(args); | ||
| int backlog; | ||
| if (!args[2]->Int32Value(env->context()).To(&backlog)) return; |
There was a problem hiding this comment.
i made an update here, so i think we should validate here instead of throwing args error
There was a problem hiding this comment.
you have this setting to backlog as an int32 but we use it as port below, cast as a uint32 then converted to an int with port below. Where is the term backlog coming from here? either way, should make this consistent here.
There was a problem hiding this comment.
yep, i think the backlog should be a "port" here, and I think we don't need cast again since
int port;
if (!args[2]->Int32Value(env->context()).To(&port)) return;
already casted to int right (?)
and we don't need this anymore
int port = static_cast<int>(args[2].As<Uint32>()->Value());
8bfc0fb to
5db5ac6
Compare
| Environment* env = Environment::GetCurrent(args); | ||
| int port; | ||
| if (!args[2]->Int32Value(env->context()).To(&port)) return; | ||
|
|
jazelly
left a comment
There was a problem hiding this comment.
can you please also add a test?
|
|
||
| #include <cstdlib> | ||
|
|
||
|
|
There was a problem hiding this comment.
can you please remove these unnecessary changes
There was a problem hiding this comment.
I have added a unit test, but the unnecessary changes are clearly a result of the formatter overzealous behavior. as @jasnell said above, I restored the changes to their original state, yet every time I save the file, it reverts to the modified version.
5db5ac6 to
24f0f12
Compare
24f0f12 to
707c925
Compare
|
Hi guys, I need help for re-review my PR here :) |
|
This pull request has been marked as stale due to 210 days of inactivity. |
|
Hello, I'm doing some backlog hygiene on all PRs related to networking and streaming modules. There hasn't been any activity here for quite a while so I'm going to mark it Thank you! |
|
This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open. |
Problem:
When fewer than three arguments are passed or if the third argument is not of type Uint32, the program attempts to access invalid memory or cast incorrect types. This leads to undefined behavior, including potential crashes
[1] 2385420 IOT instruction (core dumped) nodeand not clear what does it meanSo I think we need to validate the args first before we do some assertions below. we can also do same thing in another place that doesn't have the validator
ref issue: #56367