eslint-plugin-unicorn icon indicating copy to clipboard operation
eslint-plugin-unicorn copied to clipboard

Rule proposal: Forbid `btoa` and `atob`

Open fregante opened this issue 2 years ago • 12 comments

Description

See:

  • https://developer.mozilla.org/en-US/docs/Glossary/Base64#the_unicode_problem
  • https://github.com/sindresorhus/uint8array-extras/blob/75bd46dca10f7ec4fe1452fff01e1eec8c45be4d/index.js#L122-L123

In short, they don't support UTF-8

Fail

globalThis.btoa(str);
window.atob(str);
btoa(str);
atob(str)

Pass

import {base64ToString, stringToBase64} from "uint8array-extras";

base64ToString(str);
stringToBase64(str);

Additional Info

Hopefully more native methods will be available soon:

  • https://github.com/tc39/proposal-arraybuffer-base64

fregante avatar Oct 29 '23 16:10 fregante

I use btoa(unescape(encodeURIComponent('中文'))) a lot. It works for me.

fisker avatar Oct 29 '23 16:10 fisker

unescape() Deprecated: This feature is no longer recommended. Though some browsers might still support it, it may have already been removed from the relevant web standards, may be in the process of being dropped, or may only be kept for compatibility purposes. Avoid using it, and update existing code if possible; see the compatibility table at the bottom of this page to guide your decision. Be aware that this feature may cease to work at any time.

  • https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/unescape

sindresorhus avatar Oct 29 '23 18:10 sindresorhus

I know that, but it will still be in browser forever. I prefer to use it without add dependencies.

fisker avatar Oct 29 '23 18:10 fisker

Sure, but it's still not something we should recommend in a lint rule.

The proper solution is what uint8array-extras does, which is also documented here: https://developer.mozilla.org/en-US/docs/Glossary/Base64#the_unicode_problem

sindresorhus avatar Oct 29 '23 18:10 sindresorhus

I do agree on flagging atob and btoa but maybe it should not make any module recommendations as those are subjective. I used https://www.npmjs.com/package/base-64 in the past.

silverwind avatar Nov 02 '23 17:11 silverwind

I don't know how that compares performance-wise, but that package is bigger than the whole uint8array-utils package, so maybe it should make recommendations.

fregante avatar Nov 02 '23 18:11 fregante

Yeah I wouln't use it today anymore. Maybe it should link to that MDN until a new standard method arises.

silverwind avatar Nov 02 '23 18:11 silverwind

I think https://eslint.org/docs/latest/rules/no-restricted-globals could cover at least part of this:

"no-restricted-globals": [
    "error",
    {
        "name": "atob",
        "message": "Use uint8array-extras instead"
    },
    {
        "name": "btoa",
        "message": "Use uint8array-extras instead"
    }
]

I think it will definitely match on atob, but likely not on window.atob or globalThis.atob, but I would consider that a rule bug. Assuming eslint core will stubbornly refuse to acknowledge it as a bug, I think it's worth to create a improved rule here.

silverwind avatar Nov 02 '23 18:11 silverwind

Assuming eslint core will stubbornly refuse to acknowledge it as a bug

Open an ESLint issue. I don't see why they would refuse that.

sindresorhus avatar May 08 '24 22:05 sindresorhus

I won't, had too many bad experiences with opening issues at eslint because they always refuse any behaviour changes in their rules. At this point, I think it's better to fork and fix no-restricted-globals.

This playground confirms above issue.

silverwind avatar May 08 '24 23:05 silverwind