ronantakizawa/codereview-bench huggingface dataset:
Repository: mozilla/send
File: app/fileSender.js (JavaScript)

Code before:
      },
      true,
      ['sign']
    );
    const newAuthKey = await window.crypto.subtle.deriveKey(
      {
        name: 'PBKDF2',
        salt: encoder.encode(file.url),
        iterations: 100,
        hash: 'SHA-256'
      },
      pwdKey,
      {
        name: 'HMAC',
        hash: 'SHA-256'
      },
      true,
      ['sign']
    );
    const rawAuth = await window.crypto.subtle.exportKey('raw', newAuthKey);
    try {
      if (existingPassword) {
        await sendPassword(file, oldAuthKey, rawAuth);
      } else {
        await sendPassword(file, authKey, rawAuth);
      }
    } catch (e) {
      if (e.message === '401' && file.nonce !== e.nonce) {
        if (existingPassword) {
          await sendPassword(file, oldAuthKey, rawAuth);
        } else {
          await sendPassword(file, authKey, rawAuth);
        }
      } else {
        throw e;
      }
    }
  }
}


Review comment (security):
Just a nit on style... I'd prefer something like:
```js
const aKey = existingPassword ? oldAuthKey : authKey;
await sendPassword(file, aKey, rawAuth);
```

Code after:
      },
      true,
      ['sign']
    );
    const newAuthKey = await window.crypto.subtle.deriveKey(
      {
        name: 'PBKDF2',
        salt: encoder.encode(file.url),
        iterations: 100,
        hash: 'SHA-256'
      },
      pwdKey,
      {
        name: 'HMAC',
        hash: 'SHA-256'
      },
      true,
      ['sign']
    );
    const rawAuth = await window.crypto.subtle.exportKey('raw', newAuthKey);
    const aKey = existingPassword ? oldAuthKey : authKey;
    try {
      await sendPassword(file, aKey, rawAuth);
    } catch (e) {
      if (e.message === '401' && file.nonce !== e.nonce) {
        await sendPassword(file, aKey, rawAuth);
      } else {
        throw e;
      }
    }
  }
}
Repository: mozilla/send
File: app/fileSender.js (JavaScript)

Code before:
import Nanobus from 'nanobus';
import { arrayToB64, b64ToArray, bytes } from './utils';

async function getAuthHeader(authKey, nonce) {
  const sig = await window.crypto.subtle.sign(
    {
      name: 'HMAC'
    },
    authKey,
    b64ToArray(nonce)
  );
  return `send-v1 ${arrayToB64(new Uint8Array(sig))}`;
}

async function sendPassword(file, authKey, rawAuth) {
  const authHeader = await getAuthHeader(authKey, file.nonce);
  return new Promise((resolve, reject) => {
    const xhr = new XMLHttpRequest();
    xhr.onreadystatechange = () => {
      if (xhr.readyState === XMLHttpRequest.DONE) {
        const nonce = xhr.getResponseHeader('WWW-Authenticate').split(' ')[1];
        file.nonce = nonce;
        if (xhr.status === 200) {
          return resolve(xhr.response);
        }
        reject(new Error(xhr.status));
      }
    };
    xhr.onerror = () => reject(new Error(0));
    xhr.ontimeout = () => reject(new Error(0));
    xhr.open('post', `/api/password/${file.id}`);
    xhr.setRequestHeader('Authorization', authHeader);
    xhr.setRequestHeader('Content-Type', 'application/json');
    xhr.responseType = 'json';
    xhr.timeout = 2000;
    xhr.send(JSON.stringify({ auth: arrayToB64(new Uint8Array(rawAuth)) }));
  });
}

export default class FileSender extends Nanobus {
  constructor(file) {
    super('FileSender');
    this.file = file;
    this.msg = 'importingFile';
    this.progress = [0, 1];
    this.cancelled = false;

Review comment (suggestion):
There are cases where this header is not set (status == 404 for example), so would throw an exception here

Code after:
import Nanobus from 'nanobus';
import { arrayToB64, b64ToArray, bytes } from './utils';

async function getAuthHeader(authKey, nonce) {
  const sig = await window.crypto.subtle.sign(
    {
      name: 'HMAC'
    },
    authKey,
    b64ToArray(nonce)
  );
  return `send-v1 ${arrayToB64(new Uint8Array(sig))}`;
}

async function sendPassword(file, authKey, rawAuth) {
  const authHeader = await getAuthHeader(authKey, file.nonce);
  return new Promise((resolve, reject) => {
    const xhr = new XMLHttpRequest();
    xhr.onreadystatechange = () => {
      if (xhr.readyState === XMLHttpRequest.DONE) {
        if (xhr.status === 200) {
          const nonce = xhr.getResponseHeader('WWW-Authenticate').split(' ')[1];
          file.nonce = nonce;
          return resolve(xhr.response);
        }
        reject(new Error(xhr.status));
      }
    };
    xhr.onerror = () => reject(new Error(0));
    xhr.ontimeout = () => reject(new Error(0));
    xhr.open('post', `/api/password/${file.id}`);
    xhr.setRequestHeader('Authorization', authHeader);
    xhr.setRequestHeader('Content-Type', 'application/json');
    xhr.responseType = 'json';
    xhr.timeout = 2000;
    xhr.send(JSON.stringify({ auth: arrayToB64(new Uint8Array(rawAuth)) }));
  });
}

export default class FileSender extends Nanobus {
  constructor(file) {
    super('FileSender');
    this.file = file;
    this.msg = 'importingFile';
    this.progress = [0, 1];
    this.cancelled = false;
Repository: mozilla/send
File: app/templates/share.js (JavaScript)

Code before:
/* global EXPIRE_SECONDS */
const html = require('choo/html');
const assets = require('../../common/assets');
const notFound = require('./notFound');
const uploadPassword = require('./uploadPassword');
const selectbox = require('./selectbox');
const { allowedCopy, delay, fadeOut } = require('../utils');

function passwordComplete(state, password) {
  const el = html([
    `<div class="selectPassword">${state.translate('passwordResult', {
      password: '<pre></pre>'
    })}
    <button id="resetButton">Reset</button>
    <form id='reset-form' class="setPassword hidden" data-no-csrf>
      <input id="unlock-reset-input"
        class="unlock-input input-no-btn"
        maxlength="64"
        autocomplete="off"
        placeholder="${state.translate('unlockInputPlaceholder')}">
      <input type="submit"
        id="unlock-reset-btn"
        class="btn btn-hidden"
        value="Reset Password"/>
    </form>
    </div>`
  ]);

  el.querySelector('#resetButton').onclick = toggleResetInput;
  el.querySelector('#unlock-reset-input').oninput = inputChanged;

  const passwordOriginal = document.createElement('div');
  passwordOriginal.className = 'passwordOriginal';
  passwordOriginal.innerText = password;

  const passwordStar = document.createElement('div');
  passwordStar.className = 'passwordStar';
  passwordStar.innerText = password.replace(/./g, '●');


Review comment (suggestion):
I missed this earlier... The "Reset" text here needs to be localized. We need to add a string to `public/locales/en-US/send.ftl` and use `state.translate` like the other strings here. I also think "Change" is a better word than "Reset" in this case.

Code after:
/* global EXPIRE_SECONDS */
const html = require('choo/html');
const assets = require('../../common/assets');
const notFound = require('./notFound');
const uploadPassword = require('./uploadPassword');
const selectbox = require('./selectbox');
const { allowedCopy, delay, fadeOut } = require('../utils');

function passwordComplete(state, password) {
  const el = html([
    `<div class="selectPassword">${state.translate('passwordResult', {
      password: '<pre></pre>'
    })}
    <button id="resetButton">${state.translate('changePasswordButton')}</button>
    <form id='reset-form' class="setPassword hidden" data-no-csrf>
      <input id="unlock-reset-input"
        class="unlock-input input-no-btn"
        maxlength="64"
        autocomplete="off"
        placeholder="${state.translate('unlockInputPlaceholder')}">
      <input type="submit"
        id="unlock-reset-btn"
        class="btn btn-hidden"
        value="Reset Password"/>
    </form>
    </div>`
  ]);

  el.querySelector('#resetButton').onclick = toggleResetInput;
  el.querySelector('#unlock-reset-input').oninput = inputChanged;

  const passwordOriginal = document.createElement('div');
  passwordOriginal.className = 'passwordOriginal';
  passwordOriginal.innerText = password;

  const passwordStar = document.createElement('div');
  passwordStar.className = 'passwordStar';
  passwordStar.innerText = password.replace(/./g, '●');
Repository: mozilla/send
File: app/pasteManager.js (JavaScript)

Code before:
/* global MAXFILESIZE */
import { bytes } from './utils';

export default function(state, emitter) {
  window.addEventListener('paste', event => {
    if (state.route !== '/' || state.uploading) return;

    for (const item of event.clipboardData.items) {
      if (!item.type.includes('image')) continue;

      const file = item.getAsFile();

      if (!file) continue; // Sometimes null

      if (file.size > MAXFILESIZE) {
        // eslint-disable-next-line no-alert
        alert(state.translate('fileTooBig', { size: bytes(MAXFILESIZE) }));
        continue;
      }

      emitter.emit('upload', { file, type: 'paste' });
    }
  });
}


Review comment (suggestion):
We currently only allow one file to be uploaded at a time. How do you think we should handle that? One way would be to `return` here.

Code after:
/* global MAXFILESIZE */
import { bytes } from './utils';

export default function(state, emitter) {
  window.addEventListener('paste', event => {
    if (state.route !== '/' || state.uploading) return;

    for (const item of event.clipboardData.items) {
      if (!item.type.includes('image')) continue;

      const file = item.getAsFile();

      if (!file) continue; // Sometimes null

      if (file.size > MAXFILESIZE) {
        // eslint-disable-next-line no-alert
        alert(state.translate('fileTooBig', { size: bytes(MAXFILESIZE) }));
        continue;
      }

      emitter.emit('upload', { file, type: 'paste' });
      return; // return here since only one file is allowed to be uploaded at a time
    }
  });
}
Repository: mozilla/send
File: build/fluent_loader.js (JavaScript)

Code before:
const { FluentBundle } = require('fluent');
const fs = require('fs');

function toJSON(map) {
  return JSON.stringify(Array.from(map));
}

function merge(m1, m2) {
  const result = new Map(m1);
  for (const [k, v] of m2) {
    result.set(k, v);
  }
  return result;
}

module.exports = function(source) {
  const localeExp = /([^/]+)\/[^/]+\.ftl$/;
  const result = localeExp.exec(this.resourcePath);
  const locale = result && result[1];
  if (!locale) {
    throw new Error(`couldn't find locale in: ${this.resourcePath}`);
  }
  // load default language and "merge" contexts
  // TODO: make this configurable
  const en_ftl = fs.readFileSync(
    require.resolve('../public/locales/en-US/send.ftl'),
    'utf8'
  );
  const en = new FluentBundle('en-US');
  en.addMessages(en_ftl);
  // pre-parse the ftl
  const context = new FluentBundle(locale);
  context.addMessages(source);

Review comment (suggestion):
In Fluent, localization works best when languages are fully isolated. Rather than merging the messages, I suggest that for non en-US locales, two `FluentResource` are included in the dist file. This ensures that interpolations of strings within other strings are consistently translated, as well as that the formatting of numbers and dates corresponds to the language of the text around them. I implemented this approach in this PR.

Code after:
const { FluentResource } = require('fluent/compat');
const fs = require('fs');

function toJSON(resource) {
  return JSON.stringify(Array.from(resource));
}

module.exports = function(source) {
  const localeExp = /([^/]+)\/[^/]+\.ftl$/;
  const result = localeExp.exec(this.resourcePath);
  const locale = result && result[1];
  if (!locale) {
    throw new Error(`couldn't find locale in: ${this.resourcePath}`);
  }

  // Parse the current language's translation file.
  const locResource = FluentResource.fromString(source);
  let enResource;

  // If the current language is not en-US, also parse en-US to provide a
  // fallback for missing translations.
  if (locale !== 'en-US') {
    const en_ftl = fs.readFileSync(
      require.resolve('../public/locales/en-US/send.ftl'),
      'utf8'
    );
    enResource = FluentResource.fromString(en_ftl);
  }

  return `
module.exports = \`
if (typeof window === 'undefined') {
  var fluent = require('fluent');
Repository: mozilla/send
File: build/fluent_loader.js (JavaScript)

Code before:
const { FluentBundle } = require('fluent');
const fs = require('fs');

function toJSON(map) {
  return JSON.stringify(Array.from(map));
}

function merge(m1, m2) {
  const result = new Map(m1);
  for (const [k, v] of m2) {
    result.set(k, v);
  }
  return result;
}

module.exports = function(source) {
  const localeExp = /([^/]+)\/[^/]+\.ftl$/;
  const result = localeExp.exec(this.resourcePath);
  const locale = result && result[1];
  if (!locale) {
    throw new Error(`couldn't find locale in: ${this.resourcePath}`);
  }
  // load default language and "merge" contexts
  // TODO: make this configurable
  const en_ftl = fs.readFileSync(
    require.resolve('../public/locales/en-US/send.ftl'),
    'utf8'
  );
  const en = new FluentBundle('en-US');
  en.addMessages(en_ftl);
  // pre-parse the ftl
  const context = new FluentBundle(locale);
  context.addMessages(source);

  const merged = merge(en._messages, context._messages);
  return `
module.exports = \`
if (typeof window === 'undefined') {
  var fluent = require('fluent');
}
(function () {
  var bundle = new fluent.FluentBundle('${locale}', {useIsolating: false});
  bundle._messages = new Map(${toJSON(merged)});
  function translate(id, data) {
    var msg = bundle.getMessage(id);
    if (typeof(msg) !== 'string' && !msg.val && msg.attrs) {
      msg = msg.attrs.title || msg.attrs.alt

Review comment (suggestion):
Ideally, we should work with an array of languages negotatied by `fluent-langneg`. I found `server/middleware/language.js` but I wasn't sure if changing `req.language` to an array was a good first step. I can look into it a bit more if you'd like.

Code after:
const { FluentResource } = require('fluent/compat');
const fs = require('fs');

function toJSON(resource) {
  return JSON.stringify(Array.from(resource));
}

module.exports = function(source) {
  const localeExp = /([^/]+)\/[^/]+\.ftl$/;
  const result = localeExp.exec(this.resourcePath);
  const locale = result && result[1];
  if (!locale) {
    throw new Error(`couldn't find locale in: ${this.resourcePath}`);
  }

  // Parse the current language's translation file.
  const locResource = FluentResource.fromString(source);
  let enResource;

  // If the current language is not en-US, also parse en-US to provide a
  // fallback for missing translations.
  if (locale !== 'en-US') {
    const en_ftl = fs.readFileSync(
      require.resolve('../public/locales/en-US/send.ftl'),
      'utf8'
    );
    enResource = FluentResource.fromString(en_ftl);
  }

  return `
module.exports = \`
if (typeof window === 'undefined') {
  var fluent = require('fluent');
}
(function () {
  let bundles = [
    ['${locale}', ${toJSON(locResource)}],
    ${enResource ? `['en-US', ${toJSON(enResource)}]` : ''}
  ].map(([locale, entries]) => {
    let bundle = new fluent.FluentBundle(locale, {useIsolating: false});
    bundle.addResource(new fluent.FluentResource(entries));
    return bundle;
  });

  function translate(id, data) {
    for (let bundle of bundles) {
      if (bundle.hasMessage(id)) {
Repository: mozilla/send
File: build/fluent_loader.js (JavaScript)

Code before:
  // TODO: make this configurable
  const en_ftl = fs.readFileSync(
    require.resolve('../public/locales/en-US/send.ftl'),
    'utf8'
  );
  const en = new FluentBundle('en-US');
  en.addMessages(en_ftl);
  // pre-parse the ftl
  const context = new FluentBundle(locale);
  context.addMessages(source);

  const merged = merge(en._messages, context._messages);
  return `
module.exports = \`
if (typeof window === 'undefined') {
  var fluent = require('fluent');
}
(function () {
  var bundle = new fluent.FluentBundle('${locale}', {useIsolating: false});
  bundle._messages = new Map(${toJSON(merged)});
  function translate(id, data) {
    var msg = bundle.getMessage(id);
    if (typeof(msg) !== 'string' && !msg.val && msg.attrs) {
      msg = msg.attrs.title || msg.attrs.alt
    }
    return bundle.format(msg, data);
  }
  if (typeof window === 'undefined') {
    module.exports = translate;
  }
  else {
    window.translate = translate;
  }
})();
\``;
};


Review comment (suggestion):
I didn't see any messages in the current FTL file which use `title` or `alt`. We're going to change the `format` API soon to allow formatting attributes, too (probably via `format("string-id.attribute-name")`). I only implemented formatting message values here. If formatting attributes is required now, I can add it in this PR. Otherwise, I suggest to wait for the next release of `fluent`.

Code after:
      require.resolve('../public/locales/en-US/send.ftl'),
      'utf8'
    );
    enResource = FluentResource.fromString(en_ftl);
  }

  return `
module.exports = \`
if (typeof window === 'undefined') {
  var fluent = require('fluent');
}
(function () {
  let bundles = [
    ['${locale}', ${toJSON(locResource)}],
    ${enResource ? `['en-US', ${toJSON(enResource)}]` : ''}
  ].map(([locale, entries]) => {
    let bundle = new fluent.FluentBundle(locale, {useIsolating: false});
    bundle.addResource(new fluent.FluentResource(entries));
    return bundle;
  });

  function translate(id, data) {
    for (let bundle of bundles) {
      if (bundle.hasMessage(id)) {
        let message = bundle.getMessage(id);
        return bundle.format(message, data);
      }
    }
  }

  if (typeof window === 'undefined') {
    module.exports = translate;
  }
  else {
    window.translate = translate;
  }
})();
\``;
};
Repository: mozilla/send
File: android/android.js (JavaScript)

Code before:
      >
        <a
          id="hamburger"
          class="absolute pin-t pin-r z-50"
          href="#"
          onclick="${clickPreferences}"
        >
          <img src="${assets.get('preferences.png')}" />
        </a>
        ${state.cache(Header, 'header').render()} ${main(state, emit)}
      </body>
    `;

    function clickPreferences(event) {
      event.preventDefault();
      emit('pushState', '/preferences');
    }
  };
}
(async function start() {
  const translate = await getTranslator('en-US');
  app.use(async (state, emitter) => {
    state.translate = translate;
    state.capabilities = {
      account: true
    }; //TODO
    state.storage = storage;
    state.user = new User(storage);
    state.raven = Raven;

    window.finishLogin = async function(accountInfo) {
      await state.user.finishLogin(accountInfo);
      emitter.emit('render');
    };

    // for debugging
    window.appState = state;
    window.appEmit = emitter.emit.bind(emitter);
  });
  app.route('/', body(home));
  app.route('/upload', upload);
  app.route('/share/:id', share);
  app.route('/preferences', preferences);
  app.route('/error', error);
  //app.route('/debugging', require('./pages/debugging').default);
  // add /api/filelist
  app.mount('body');
})();


Review comment (suggestion):
if we can set the `navigator.language` for the webview we ought to be able to get l10n here for free

Code after:
function body(main) {
  return function(state, emit) {
    return html`
      <body
        class="flex flex-col items-center font-sans bg-blue-lightest h-screen"
      >
        <a
          id="hamburger"
          class="absolute pin-t pin-r z-50"
          href="#"
          onclick="${clickPreferences}"
        >
          <img src="${assets.get('preferences.png')}" />
        </a>
        ${state.cache(Header, 'header').render()} ${main(state, emit)}
      </body>
    `;

    function clickPreferences(event) {
      event.preventDefault();
      emit('pushState', '/preferences');
    }
  };
}
(async function start() {
  const translate = await getTranslator('en-US');
  app.use(async (state, emitter) => {
    state.translate = translate;
    state.capabilities = {
      account: true
    }; //TODO
    state.storage = storage;
    state.user = new User(storage);
    state.raven = Raven;

    window.finishLogin = async function(accountInfo) {
      await state.user.finishLogin(accountInfo);
      emitter.emit('render');
    };

    // for debugging
    window.appState = state;
    window.appEmit = emitter.emit.bind(emitter);
  });
  app.route('/', body(home));
  app.route('/upload', upload);
  app.route('/share/:id', share);
  app.route('/preferences', preferences);
  app.route('/error', error);
  //app.route('/debugging', require('./pages/debugging').default);
  // add /api/filelist
Repository: mozilla/send
File: app/ui/archiveTile.js (JavaScript)

Code before:
      >
        <summary
          >${
            translate('fileCount', {
              num: archive.manifest.files.length
            })
          }</summary
        >
        ${
          list(
            archive.manifest.files.map(f => fileInfo(f)),
            'list-reset h-full'
          )
        }
      </details>
    `;
  }
  function toggled(event) {
    event.stopPropagation();
    archive.open = event.target.open;
  }
}

module.exports = function(state, emit, archive) {
  const copyOrShare =
    typeof window.Android !== 'object'
      ? html`
          <button
            class="text-blue hover:text-blue-dark focus:text-blue-darker self-end font-medium flex items-center"
            onclick=${copy}
          >
            <img src="${assets.get('copy-16.svg')}" class="mr-2" /> ${
              state.translate('copyUrlHover')
            }
          </button>
        `
      : html`
          <button
            class="text-blue hover:text-blue-dark focus:text-blue-darker self-end font-medium flex items-center"
            onclick=${share}
          >
            <img src="${assets.get('share-16.svg')}" class="mr-2" /> Share
          </button>
        `;
  return html`
  <send-archive
    id="archive-${archive.id}"
    class="flex flex-col items-start border border-grey-light bg-white p-4 w-full">
    <p class="w-full">
      <img class="float-left mr-3" src="${assets.get('blue_file.svg')}"/>
      <input

Review comment (suggestion):
we should standardize how we check for android. I used `utils/browserName` somewhere else to do the same thing. I don't really care how we do it but we should probably wrap it in a function.

Code after:
function archiveDetails(translate, archive) {
  if (archive.manifest.files.length > 1) {
    return html`
      <details
        class="w-full pb-1 overflow-y-scroll"
        ${archive.open ? 'open' : ''}
        ontoggle="${toggled}"
      >
        <summary
          >${
            translate('fileCount', {
              num: archive.manifest.files.length
            })
          }</summary
        >
        ${
          list(
            archive.manifest.files.map(f => fileInfo(f)),
            'list-reset h-full'
          )
        }
      </details>
    `;
  }
  function toggled(event) {
    event.stopPropagation();
    archive.open = event.target.open;
  }
}

module.exports = function(state, emit, archive) {
  const copyOrShare =
    browserName() !== 'android-app'
      ? html`
          <button
            class="text-blue hover:text-blue-dark focus:text-blue-darker self-end font-medium flex items-center"
            onclick=${copy}
          >
            <img src="${assets.get('copy-16.svg')}" class="mr-2" /> ${
              state.translate('copyUrlHover')
            }
          </button>
        `
      : html`
          <button
            class="text-blue hover:text-blue-dark focus:text-blue-darker self-end font-medium flex items-center"
            onclick=${share}
          >
            <img src="${assets.get('share-16.svg')}" class="mr-2" /> Share
          </button>
        `;