From 641b11aaafedccf960fc160bb4ac8519dec5b1a2 Mon Sep 17 00:00:00 2001
From: Clement Escoffier <clement.escoffier@gmail.com>
Date: Thu, 25 Jul 2024 15:26:21 +0200
Subject: [PATCH] Redis - Improve parameter validation of the hmget command

Fix #42131
---
 .../redis/datasource/hash/HashCommands.java   |  4 ++--
 .../datasource/AbstractHashCommands.java      |  5 +++++
 .../redis/datasource/HashCommandsTest.java    | 21 +++++++++++++++++++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/extensions/redis-client/runtime/src/main/java/io/quarkus/redis/datasource/hash/HashCommands.java b/extensions/redis-client/runtime/src/main/java/io/quarkus/redis/datasource/hash/HashCommands.java
index d71e80ae867e2..beeb6bb39c022 100644
--- a/extensions/redis-client/runtime/src/main/java/io/quarkus/redis/datasource/hash/HashCommands.java
+++ b/extensions/redis-client/runtime/src/main/java/io/quarkus/redis/datasource/hash/HashCommands.java
@@ -122,8 +122,8 @@ public interface HashCommands<K, F, V> extends RedisCommands {
      * Group: hash
      * Requires Redis 2.0.0
      *
-     * @param key the key
-     * @param fields the fields
+     * @param key the key must not be {@code null}
+     * @param fields the fields, must not be empty, must not contain {@code null} values
      * @return list of values associated with the given fields, in the same order as they are requested.
      *         If a requested field does not exist, the returned map contains a {@code null} value for that field.
      **/
diff --git a/extensions/redis-client/runtime/src/main/java/io/quarkus/redis/runtime/datasource/AbstractHashCommands.java b/extensions/redis-client/runtime/src/main/java/io/quarkus/redis/runtime/datasource/AbstractHashCommands.java
index f6acc5437ed6a..63402a8227c30 100644
--- a/extensions/redis-client/runtime/src/main/java/io/quarkus/redis/runtime/datasource/AbstractHashCommands.java
+++ b/extensions/redis-client/runtime/src/main/java/io/quarkus/redis/runtime/datasource/AbstractHashCommands.java
@@ -82,6 +82,11 @@ Uni<Response> _hlen(K key) {
 
     @SafeVarargs
     final Uni<Response> _hmget(K key, F... fields) {
+        nonNull(key, "key");
+        doesNotContainNull(fields, "fields");
+        if (fields.length == 0) {
+            throw new IllegalArgumentException("`fields` must not be empty");
+        }
         RedisCommand cmd = RedisCommand.of(Command.HMGET);
         cmd.put(marshaller.encode(key));
 
diff --git a/extensions/redis-client/runtime/src/test/java/io/quarkus/redis/datasource/HashCommandsTest.java b/extensions/redis-client/runtime/src/test/java/io/quarkus/redis/datasource/HashCommandsTest.java
index 3d09274485c92..ca9ed371e2554 100644
--- a/extensions/redis-client/runtime/src/test/java/io/quarkus/redis/datasource/HashCommandsTest.java
+++ b/extensions/redis-client/runtime/src/test/java/io/quarkus/redis/datasource/HashCommandsTest.java
@@ -1,6 +1,7 @@
 package io.quarkus.redis.datasource;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.entry;
 import static org.assertj.core.api.Assertions.offset;
 
@@ -392,4 +393,24 @@ void populateManyEntries(HashCommands<String, String, String> hash, Map<String,
         hash.hset(key, expect);
     }
 
+    /**
+     * Reproducer for <a href="https://github.com/quarkusio/quarkus/issues/42131">#42131</a>.
+     */
+    @Test
+    void testInvalidHashMGet() {
+        HashCommands<String, String, String> cmd = ds.hash(String.class, String.class, String.class);
+        // Key must not be null
+        assertThatThrownBy(() -> cmd.hmget(null, "a", "b")).isInstanceOf(IllegalArgumentException.class)
+                .hasMessageContaining("key");
+        // Fields must not be empty
+        assertThatThrownBy(() -> cmd.hmget("key")).isInstanceOf(IllegalArgumentException.class)
+                .hasMessageContaining("fields");
+
+        // Fields must not contain `null`
+        assertThatThrownBy(() -> cmd.hmget("key", null, "b")).isInstanceOf(IllegalArgumentException.class)
+                .hasMessageContaining("fields");
+        assertThatThrownBy(() -> cmd.hmget("key", "a", null, "b")).isInstanceOf(IllegalArgumentException.class)
+                .hasMessageContaining("fields");
+    }
+
 }