Added alphabet check before decoding #7

开启中
Ghost 请求将 3 次代码提交从 (已删除): master 合并至 master
首次贡献者

Resolves #6

Linear time complexity solution, guaranteed to work.

Alternatively you could check the expected length of the output matches but then you will have to deal with edge cases (such as the test case given).

Resolves #6 Linear time complexity solution, guaranteed to work. Alternatively you could check the expected length of the output matches but then you will have to deal with edge cases (such as the test case given).
Ghost2021-03-04 05:12:50 +00:00 推送了 1 个提交
管理员

I'm open to updating this, but it's so old an relied on so heavily by so many projects and devices (including the AppleTV SDK) that I think I'll have to do a major version bump even for a bugfix.

You could actually make this quite a bit faster by checking character ranges.

// off the cuff, not correct code
if (!(x >= "a".toCharCode() && x <= "Z".toCharCode() /* && range of symbols*/)) {
   throw Error('barf');
}

Also, I want to stick with Vanilla JavaScript (ES5) so that, for those who update, they don't have to introduce a compiler or change their other build tools.

I'm open to updating this, but it's so old an relied on so heavily by so many projects and devices (including the AppleTV SDK) that I think I'll have to do a major version bump even for a bugfix. You could actually make this quite a bit faster by checking character ranges. ```js // off the cuff, not correct code if (!(x >= "a".toCharCode() && x <= "Z".toCharCode() /* && range of symbols*/)) { throw Error('barf'); } ``` Also, I want to stick with Vanilla JavaScript (ES5) so that, for those who update, they don't have to introduce a compiler or change their other build tools.
Ghost2021-03-11 13:26:24 +00:00 推送了 1 个提交
Ghost2021-03-11 13:27:27 +00:00 推送了 1 个提交
作者
首次贡献者

I changed it to a range check (good advice).

I think I've changed it to ES5, but I'm not super familiar with what is ES5 and what came after so I could have missed things.

We use a package called localflare to run Cloudflare Workers for testing which depends on this package. I discovered the different behavior between atob and buffer when looking at b64url in our code.

For us making the change is beneficial because the closer we can get our environment to Cloudflare the better (and I'm really against Node's behavior on this) but I also understand being cautious about versioning. I'll just have to make a PR to localflare to update their version :)

(Wrangler is on our roadmap but it will be a little while before we get there)

I changed it to a range check (good advice). I think I've changed it to ES5, but I'm not super familiar with what is ES5 and what came after so I could have missed things. We use a package called localflare to run Cloudflare Workers for testing which depends on this package. I discovered the different behavior between `atob` and `buffer` when looking at b64url in our code. For us making the change is beneficial because the closer we can get our environment to Cloudflare the better (and I'm really against Node's behavior on this) but I also understand being cautious about versioning. I'll just have to make a PR to localflare to update their version :) (Wrangler is on our roadmap but it will be a little while before we get there)
管理员

Thanks.
I've scheduled review, update, merge, and publish this on Saturday.

Thanks. I've scheduled review, update, merge, and publish this on Saturday.
此合并请求因为派生仓库信息缺失而中断。
登录 并参与到对话中。
无审核者
未选择标签
未选择里程碑
未指派成员
2 名参与者
通知
到期时间
到期日期无效或超出范围。请使用 'yyyy-mm-dd' 格式。

未设置到期时间。

依赖工单

没有设置依赖项。

参考:coolaj86/atob.js#7
没有提供说明。