Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call setTimeout with function expression. #155

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

diegocr
Copy link

@diegocr diegocr commented Sep 7, 2015

In order to prevent vulnerabilities, the setTimeout and setInterval functions should be called only with function expressions as their first argument.

In order to prevent vulnerabilities, the `setTimeout` and `setInterval` functions should be called only with function expressions as their first argument.
@dmethvin
Copy link
Member

I'm not sure I understand. I think you're referring to the use of a string as the first arg, which doesn't apply here. Can you point to a reference that supports your suggestion?

@diegocr
Copy link
Author

diegocr commented Sep 10, 2015

Hi @dmethvin

I'm submitting an add-on to the Mozilla website (addons.mozilla.org) containing your lib, and their code validator is complaining with the following:

Warning: `setTimeout` called in potentially dangerous manner
In order to prevent vulnerabilities, the `setTimeout` and `setInterval` functions should be called only with function expressions as their first argument.
    Automated signing severity: high
    Suggestions for passing automated signing:
Please do not ever call `setTimeout` or `setInterval` with string arguments. If you are passing a function which is not being correctly detected as such, please consider passing a closure or arrow function, which in turn calls the original function.
    Tier:   3
    File:   js/vendor/jquery.mousewheel.js
    Line:   181
    Column: 33
    Context:
    > if (nullLowestDeltaTimeout) { clearTimeout(nullLowestDeltaTimeout); }
    > nullLowestDeltaTimeout = setTimeout(nullLowestDelta, 200);
    > 

Apparently the main concern here is that the validator has no reliably way of knowing if nullLowestDelta is really a function or a string (or if it could turn into a string at some point)

@dmethvin
Copy link
Member

Okay, so it's a limitation in their tool. I don't see any harm in this change and would have done it this way to begin with, so LGTM. I just wanted to be sure I understood why the change was being made.

@diegocr
Copy link
Author

diegocr commented Sep 11, 2015

Thank you! (as long LGTM stand for "let's get this merged" :))

Seriously, the Mozilla Editors don't allow including modified vendor files in extensions, hence this getting merged here will make their and my life easier.

Base automatically changed from master to main March 16, 2021 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants