[v14.x backport] buffer: add base64url encoding option#39702
[v14.x backport] buffer: add base64url encoding option#39702targos wants to merge 2 commits intonodejs:v14.x-stagingfrom
Conversation
github-actions
bot
added
c++
github-actions
bot
removed
the
request-ci
|
There was also this doc fix fd02dac you could pick |
|
fd02dac is already in v14.x |
| enum encoding {ASCII, UTF8, BASE64, UCS2, BINARY, HEX, BUFFER, LATIN1 = BINARY}; | ||
| // BASE64URL is not currently exposed to the JavaScript side. | ||
| enum encoding { | ||
| ASCII, | ||
| UTF8, | ||
| BASE64, | ||
| UCS2, | ||
| BINARY, | ||
| HEX, | ||
| BUFFER, | ||
| BASE64URL, | ||
| LATIN1 = BINARY | ||
| }; |
There was a problem hiding this comment.
@addaleax IIUC, this is OK ABI-wise because the new element doesn't change the other values of the enum? But ParseEncoding can now return this new value and programs that don't expect it would break? Can we do something about it (other than not backporting the feature)?
There was a problem hiding this comment.
@targos Yes, exactly, it’s ABI-compatible but not API-compatible, strictly speaking, and I don’t think we could really do something about it other than modifying ParseEncoding() to not return that value when called from userland code.
The problem with that is that, since we fall back to a default encoding in that case, ParseEncoding(isolate, "base64url") == LATIN1, which would seem like a bug.
I’d probably just leave this as-is, it’s unlikely to cause problems for a lot of addons (and even then only when somebody actually uses base64url with it).
BethGriggs
force-pushed
the
v14.x-staging
branch
from
78bb65e to
73e6781
Compare
PR-URL: nodejs#36952 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Backport parts of nodejs@dae283d
Backport parts of dae283d PR-URL: #36952 Backport-PR-URL: #39702 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
|
Landed in a343956 |
Backport parts of dae283d PR-URL: #36952 Backport-PR-URL: #39702 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Backport parts of nodejs@dae283d PR-URL: nodejs#36952 Backport-PR-URL: nodejs#39702 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
4 participants
The fixup commit is a partial application of the changes from dae283d that are needed for this to work.