Hi Florents,
> Here's a v3 with some (hopefully) better test cases.
Thanks for the new version of the patch.
```
+ encoded_len = pg_base64_encode(src, len, dst);
+
+ /* Convert Base64 to Base64URL */
+ for (uint64 i = 0; i < encoded_len; i++) {
+ if (dst[i] == '+')
+ dst[i] = '-';
+ else if (dst[i] == '/')
+ dst[i] = '_';
+ }
```
Although it is a possible implementation, wouldn't it be better to
parametrize pg_base64_encode instead of traversing the string twice?
Same for pg_base64_decode. You can refactor pg_base64_encode and make
it a wrapper for pg_base64_encode_impl if needed.
```
+-- Flaghsip Test case against base64.
+-- Notice the = padding removed at the end and special chars.
+SELECT encode('\x69b73eff', 'base64'); -- Expected: abc+/w==
+ encode
+----------
+ abc+/w==
+(1 row)
+
+SELECT encode('\x69b73eff', 'base64url'); -- Expected: abc-_w
+ encode
+--------
+ abc-_w
+(1 row)
```
I get the idea, but calling base64 is redundant IMO. It only takes
several CPU cycles during every test run without much value. I suggest
removing it and testing corner cases for base64url instead, which is
missing at the moment. Particularly there should be tests for
encoding/decoding strings of 0/1/2/3/4 characters and making sure that
decode(encode(x)) = x, always. On top of that you should cover with
tests the cases of invalid output for decode().
--
Best regards,
Aleksander Alekseev
