FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

buffer,lib: update atob to align wpt's base64.json by XadillaX · Pull Request #43901 · nodejs/node · GitHub

/ node Public

buffer,lib: update atob to align wpt's base64.json - #43901

Closed
XadillaX wants to merge 6 commits into
nodejs:mainfrom
XadillaX:fix-atob
ConversationCommits (6)ChecksFiles changed
Closed

buffer,lib: update atob to align wpt's base64.json#43901
XadillaX wants to merge 6 commits into
nodejs:mainfrom
XadillaX:fix-atob

Conversation

XadillaX commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. needs-ci PRs that need a full CI run. labels Jul 19, 2022
aduh95 reviewed Jul 19, 2022
View reviewed changes

aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't think we need to remove the equal signs from the string, couldn't we simply count them and throw if there's more than two or if a different char after the first equal sign?

Comment thread lib/buffer.js Outdated
Comment thread lib/buffer.js Outdated
aduh95 reviewed Jul 28, 2022
View reviewed changes
Comment thread lib/buffer.js Outdated
aduh95 reviewed Jul 28, 2022
View reviewed changes
Comment thread lib/buffer.js Outdated
aduh95 reviewed Jul 28, 2022
View reviewed changes
Comment thread lib/buffer.js Outdated
aduh95 reviewed Jul 30, 2022
View reviewed changes
Comment thread lib/buffer.js Outdated

aduh95 Jul 30, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If you put this check here, you don't need to repeat the throw lazyDOMException part, right?

Suggested change
if (index === kEqualSignIndex) {
if (index === kEqualSignIndex && equalCharCount !== 2) {

Other than that, LGTM

aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 30, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 30, 2022

nodejs-github-bot commented Aug 30, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/46320/

This was referenced Aug 31, 2022
Open
Open
Open
Open

nodejs-github-bot commented Oct 8, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47123/

nodejs-github-bot commented Oct 8, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47124/

nodejs-github-bot commented Oct 8, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47125/

nodejs-github-bot commented Oct 8, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47126/

XadillaX added a commit that referenced this pull request Oct 9, 2022
buffer,lib: update atob to align wpt's base64.json
3a7fcdc 
Refs: https://html.spec.whatwg.org/multipage/webappapis.html#dom-atob-dev
Refs: https://infra.spec.whatwg.org/#forgiving-base64-decode
PR-URL: #43901
Reviewed-By: James M Snell <jasnell@gmail.com>

XadillaX commented Oct 9, 2022

Copy link
Copy Markdown
Contributor Author

Landed in 3a7fcdc

XadillaX closed this Oct 9, 2022
github-actions Bot mentioned this pull request Oct 10, 2022
Open
18 tasks
github-actions Bot mentioned this pull request Oct 11, 2022
Open
15 tasks
danielleadams pushed a commit that referenced this pull request Oct 11, 2022
buffer,lib: update atob to align wpt's base64.json
653c3b1 
Refs: https://html.spec.whatwg.org/multipage/webappapis.html#dom-atob-dev
Refs: https://infra.spec.whatwg.org/#forgiving-base64-decode
PR-URL: #43901
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams mentioned this pull request Oct 11, 2022
Merged
github-actions Bot mentioned this pull request Oct 12, 2022
Open
19 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

aduh95 aduh95 left review comments
jasnell jasnell approved these changes

Assignees

No one assigned

Labels

buffer Issues and PRs related to the buffer subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL