Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/sie_ts_sdk/src/images.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,12 @@ export async function toImageBytes(input: ImageInput): Promise<Uint8Array> {

// Base64 string or data URL
if (typeof input === "string") {
// Check if it's a data URL
const dataUrlMatch = input.match(/^data:[^;]+;base64,(.+)$/);
// Check if it's a base64 data URL. Per RFC 2397 the media type may carry
// parameters (e.g. ";charset=utf-8") or be omitted entirely, so match
// everything up to the ";base64," marker rather than a single ";"-free
// segment — otherwise such URLs fall through and the whole data URL is
// handed to the base64 decoder (corrupting the bytes or throwing).
const dataUrlMatch = input.match(/^data:[^,]*;base64,(.+)$/);

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.

Could we avoid encoding the data-URL grammar in a regex here? Since the format has a well-defined comma delimiter, a small helper could find the first comma and inspect the semicolon-delimited metadata before it for the final base64 marker. That would be easier to follow and give us a natural place to handle the scheme and marker case-insensitively, percent-decode the payload, and return a clear error for malformed data: input.

As written, inputs such as DATA:image/png;BASE64,SGVsbG8%3D still miss the match and are passed to the raw-base64 decoder. Would you be open to extracting something like parseBase64DataUrl(input): string | undefined, leaving base64ToBytes responsible only for decoding the extracted payload?

if (dataUrlMatch?.[1]) {
return base64ToBytes(dataUrlMatch[1]);
}
Expand Down
19 changes: 19 additions & 0 deletions packages/sie_ts_sdk/tests/images.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,25 @@ describe("toImageBytes", () => {
expect(new TextDecoder().decode(result)).toBe("test");
});

it("decodes a data URL whose media type carries a parameter", async () => {
// Valid per RFC 2397: the media type may be followed by ";param=value"
// (e.g. charset) before ";base64,". "Hello" base64-encoded.
const dataUrl = "data:image/svg+xml;charset=utf-8;base64,SGVsbG8=";

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.

These two regression cases are a good start. If we switch to a small parser, could we also cover an uppercase scheme/BASE64 marker, percent-escaped padding such as %3D, and one explicit empty-payload case? Those are the boundaries where the current regex falls through into the raw-base64 path, and the tests would make the intended behavior clear.

const result = await toImageBytes(dataUrl);

expect(result).toBeInstanceOf(Uint8Array);
expect(new TextDecoder().decode(result)).toBe("Hello");
});

it("decodes a data URL with an omitted media type", async () => {
// RFC 2397 permits an empty media type (defaults to text/plain).
const dataUrl = "data:;base64,SGVsbG8=";
const result = await toImageBytes(dataUrl);

expect(result).toBeInstanceOf(Uint8Array);
expect(new TextDecoder().decode(result)).toBe("Hello");
});

it("throws for unsupported input type", async () => {
await expect(toImageBytes(123 as unknown as Uint8Array)).rejects.toThrow(
"Unsupported image input type",
Expand Down