Closed Bug 1508825 Opened 5 years ago Closed 5 years ago

Enable ESLint for dom/crypto


(Core :: DOM: Security, enhancement, P2)




Tracking Status
firefox65 --- wontfix
firefox66 --- fixed


(Reporter: standard8, Assigned: klymenkodp)



(Whiteboard: [seneca-eslint][domsecurity-active])


(2 files)

As part of rolling out ESLint across the tree, we should enable it for dom/crypto.
I would be happy to take this issue.
Assignee: standard8 → klymenkodp
Priority: -- → P2
Whiteboard: [seneca-eslint] → [seneca-eslint][domsecurity-active]
I am confused with the following eslint error:
Function 'next' expected no return value.  consistent-return (test_WebCrypto_HKDF.html, 159:7)

This is the function:

function next() {
      if (!tests.length) {

      var test = tests.shift();
      var {key} = test;

      return crypto.subtle.importKey("raw", key, "HKDF", false, ["deriveBits"])
        .then(function(baseKey) {
          return crypto.subtle.deriveBits({
            name: "HKDF",
            hash: test.prf,
            salt: test.salt,
          }, baseKey, * 8);
        .then(function(data) {
          if (!util.memcmp(data, {
            throw new Error("derived bits don't match expected value");

          // Next test vector.
          return next();

I thought the problem was with the first conditional statement: "if (!tests.length) { return; }" 
When I remove "return;" , there is no eslint error, but the test fails.
The issue here is the inconsistent return value. So the one in the if statement returns nothing, but the main one (`return crypto.subtle.importKey`) is likely returning a promise (you can tell this due to the '.then' calls after it.

Also notice that in the last .then section, next() is called itself, iteratively.

So the !tests.length check is to end the iteration.

I think the best way to solve this is to get the if statement to return an already resolved promise:

if (!test.length) {
  return Promise.resolve();

That will give a consistent return value, and will complete correctly.

The only slight compromise here, is that if `tests` is empty at the start of the test, then it will no longer return undefined, and so the call below the function definition (`next().then(complete(that), error(that));`) won't fail.

To work around this, we can also add an explicit check for length just after tests is defined:

var tests = tv.hkdf.slice();
if (!tests.length) {
  error(that)("No tests found");

function next() {

Note: the error call is slightly awkward due to how the tests are constructed, but I've tested it and it does work.
Pushed by
Enable ESLint for dom/crypto/ (automatic changes) r=Standard8,Ehsan
Enable ESLint for dom/crypto (manual changes) r=Standard8,Ehsan
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Volodymyr, thank you for all your work on this. It is now landed & incorporated into the source code, it is great to get this enabled.
You need to log in before you can comment on or make changes to this bug.