Re: encode/decode support for base64url

David E. Wheeler <david@justatheory.com>

From: "David E. Wheeler" <david@justatheory.com>
To: Florents Tselai <florents.tselai@gmail.com>
Cc: Aleksander Alekseev <aleksander@tigerdata.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Cary Huang <cary.huang@highgo.ca>, Daniel Gustafsson <daniel@yesql.se>, Przemysław Sztoch <przemyslaw@sztoch.pl>
Date: 2025-07-12T19:40:16Z
Lists: pgsql-hackers

Attachments

On Jul 11, 2025, at 04:26, Florents Tselai <florents.tselai@gmail.com> wrote:

>  Attached 

Thank you! This looks great. The attached revision makes a a couple of minor changes:

* Change the line wrap of the docs to be more like the rest of func.sgml
* Remove an unnecessary nested if statement
* Removed `==` from one of the test comments
* Ran pgindent to create the attached patch

A few other brief comments, entirely stylistic:

* I kind of expected pg_base64url_encode to appear immediate after pg_base64_encode. In other words, to see the two uses of pg_base64_encode_internal adjacent to each other. I think that’s more typical of the project standard. Same for the functions that call pg_base64_decode_internal.

* There are a few places where variable definition has been changed without changing the meaning, for example:

- const char *srcend = src + len,
- *s = src;
+ const char *srcend = src + len;
+ const char *s = src;

  Even if this is desirable, it might make sense to defer pure formatting changes to a separate patch.

* You define return variables in functions like pg_base64url_enc_len rather than just returning the outcome of an expression. The latter is what I see in pg_base64_enc_len, so I think would be more consistent. Io other words:

```patch
--- a/src/backend/utils/adt/encode.c
+++ b/src/backend/utils/adt/encode.c
@@ -470,8 +470,6 @@ pg_base64_dec_len(const char *src, size_t srclen)
 static uint64
 pg_base64url_enc_len(const char *src, size_t srclen)
 {
-	uint64		result;
-
 	/*
 	 * Base64 encoding converts 3 bytes into 4 characters Formula: ceil(srclen
 	 * / 3) * 4
@@ -479,10 +477,8 @@ pg_base64url_enc_len(const char *src, size_t srclen)
 	 * Unlike standard base64, base64url doesn't use padding characters when
 	 * the input length is not divisible by 3
 	 */
-	result = (srclen + 2) / 3 * 4;	/* ceiling division by 3, then multiply by
+	return (srclen + 2) / 3 * 4;	/* ceiling division by 3, then multiply by
 									 * 4 */
-
-	return result;
 }
 
 static uint64
```

I suspect these are the sorts of things a committer would tweak/adjust before committing, just thinking about getting ahead of that. I think it’s ready.

Best,

David