JavaScript: Documentation review for new query UnvalidatedDynamicMethodCall.#559
Conversation
|
@xiemaisi - apologies, I wasn't well yesterday and called in sick. Will review this now. |
mchammer01
left a comment
mchammer01
left a comment
There was a problem hiding this comment.
@xiemaisi - this looks good. I noticed a typo and suggested some minor punctuation changes. See inline comments for details.
| exception. If this exception is not handled, it could be used to mount a denial-of-service attack. | ||
| </p> | ||
| <p> | ||
| For example, there might not be a method of the given name or the result of the lookup might not be |
There was a problem hiding this comment.
Line 9: "if he method name is user controlled" -> if the method name is user**-**controlled
| <p> | ||
| For example, there might not be a method of the given name or the result of the lookup might not be | ||
| a function, which would cause the method call to throw a <code>TypeError</code> at runtime. | ||
| For example, there might not be a method of the given name, or the result of the lookup might not be |
There was a problem hiding this comment.
line 35-36: add a comma for a better understanding:
Even if the object only contains methods**,** it is still a good idea to perform this check in case other properties are added to the object later on.
|
Thanks, @mc-semmle! Fixed. |
|
Thanks for your last commit @xiemaisi |
|
Are you happy for me to merge? |
|
Yes, please do; no point in waiting for the language tests. |
2 participants
#555 was (deliberately) merged without doc review to facilitate testing.