On Sun, Sep 06, 2026 at 11:22:58AM +0100, Zsolt Parragi wrote: > The recently added test extensible module has a small oversight: it > lacks input validation, which made it easy to crash the backend with > invalid input with the test module loaded. While this doesn't seem > like a critical issue for a test module, the commit message also > mentions that this could be a template for extension developers, so I > think it is worth fixing.
+static bool
+test_ext_node_expect_token(ReadNodeContext *ctx, const char *expected)
+{
+ int length;
+ const char *token = pg_strtok(ctx, &length);
+
+ return token != NULL && length == (int) strlen(expected) &&
+ memcmp(token, expected, length) == 0;
I am not much a fan of spreading pg_strtok() calls more than
necessary in this module. How about extending
test_ext_node_next_token() with an "expected" argument to enforce some
validation? If "expected" is NULL, fall back to the default of
failing if the end has been reached and a NULL token is found, for the
read callback.
+ return token != NULL && length == (int) strlen(expected) &&
+ memcmp(token, expected, length) == 0;
And perhaps also split these in multiple lines, just because it would
feel slightly cleaner.. I know I'm picky on these things.
I am not convinced that we need to check all the input function calls
in 0001 and 0002. For 0001, I'd just pick up one for simplicity of
parsing the test. I can see that you are mostly doing that in 0002,
with test_bms_num_members(). Let's just pick up one, reduce the
duplicates.
--
Michael
signature.asc
Description: PGP signature
