[WIP]Fix calling Buffer.toString with (offset, length, encoding)
#3467
Conversation
| export function utf8Slice(this: BufferExt, offset, length) { | ||
| return this.toString(offset, length, "utf8"); | ||
| export function utf8Slice(this: BufferExt, start, end) { | ||
| return this.toString("utf8", start, end); |
There was a problem hiding this comment.
start and end are used instead of offset and length to maintain compatibility with nodejs.
| expect(buf.ucs2Slice(2, 6)).toStrictEqual("いう"); | ||
| }); | ||
|
|
||
| it("Buffer.base64Slice()", () => { |
There was a problem hiding this comment.
I get a segmentfault in the base64-related tests on my local machine. I don't know what happened here.
There was a problem hiding this comment.
Attempting to debug a coredump file using gdb, the arguments passed to WTF__toBase64URLStringValue looks like strange.
Updated:
I build the latest main branch, also find this problem. toString('base64) will segment fault. But if i use the canary release bun, not find this problem.
There was a problem hiding this comment.
I vaguely remember @cirospaciari running into strange issues with .toBase64URL as well... I think we should switch to using the SIMD base64 library that Node.js also uses. Our current implementation uses a mix of WebKit's internal atob function and Zig's standard library. I would guess the cause of this is the Zig standard library usage, but it's hard to say without digging deeper
| offset = static_cast<uint32_t>(istart); | ||
| length = (length > offset) ? (length - offset) : 0; | ||
| } else { | ||
| int32_t ioffset = arg1.toInt32(lexicalGlobalObject); |
There was a problem hiding this comment.
toInt32 can throw, we should technically check for exceptions here (we did not before)
|
|
||
| expect(buf.asciiSlice()).toStrictEqual("0123456789"); | ||
| expect(buf.asciiSlice(3)).toStrictEqual("3456789"); | ||
| expect(buf.asciiSlice(3, 4)).toStrictEqual("3"); |
There was a problem hiding this comment.
These tests are failing
There was a problem hiding this comment.
oh its failing because we need to commit regenerated builtins
There was a problem hiding this comment.
Thank you. I had mistakenly thought that the generated code was not required for commit.
|
Manually tested, looks good. Thank you for this! |
|
Reading through node's implementation, do they use this interface? I don't see it 
|
|
@Jarred-Sumner |
Close: #3085
Buffer.toStringwith(offset, length, encoding)utf8Slice,latin1Slice, ...