Move base64 encoding/decoding to Rust side #13845
Conversation
| @@ -43,8 +43,8 @@ declare namespace globalThis { | |||
| result: string; | |||
| position: number; | |||
| }; | |||
| forgivingBase64Encode(data: Uint8Array): string; | |||
| forgivingBase64Decode(data: string): Uint8Array; | |||
| forgivingBase64Encode(data: string): string; | |||
There was a problem hiding this comment.
It's a bit of a shame that the strings (presumably) have to be copied. I wonder if it would be possible to have an optional &str serde?
| if char::is_ascii_whitespace(&c) { | ||
| ws = true; | ||
| } else if c == '=' { | ||
| if i != len - 1 && i != len - 2 { |
There was a problem hiding this comment.
There is a bit of a possibility for false negative here: Even if there are '=' characters at the end it does not mean that they'll be removed below since that depends on whitespace removal etc.
As such, it would be more appropriate to track '=' markers at end with an enum: None, One, Two. Then compare to those when '=' is being removed and reset the enum appropriately. If reset does not happen, it means that the '=' at the end were invalid characters.
| }); | ||
|
|
||
| // "Remove all ASCII whitespace from data" | ||
| let input = if ws { input.replace(|c| char::is_ascii_whitespace(&c), "") } else { input }; |
There was a problem hiding this comment.
It would be preferable if the replacement could be done on the same iter as the invalid characters but that's probably not possible.
| _: (), | ||
| ) -> Result<String, AnyError> { | ||
| let char_count = s.chars().count(); | ||
| let s = s.into_bytes(); | ||
| if s.len() != char_count { |
There was a problem hiding this comment.
Properly this should be s.len() > char_count but it doesn't really matter.
| @@ -190,14 +205,21 @@ fn op_base64_decode( | |||
| err | |||
| )) | |||
| })?; | |||
| Ok(ZeroCopyBuf::from(out)) | |||
| Ok(String::from_utf8(out).unwrap()) | |||
There was a problem hiding this comment.
Not sure if .unwrap() here is valid.
|
Ooops, a better perf optimization had already been posted. |
Attempt to at least somewhat fix #13838.
Before:
After: