Copy logs query to null-tolerant collections (#435)

LogsQuery may get passed in addresses in lists and topics in lists that do
not tolerate being checked for null members.  This is an unexpected and
subtle situation hence the long comment.

This new test covers this case as it passes in null-intolerant lists.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/LogsQuery.java

@@ -25,6 +25,7 @@ import org.hyperledger.besu.ethereum.core.Log;
 import org.hyperledger.besu.ethereum.core.LogTopic;
 import org.hyperledger.besu.ethereum.core.LogsBloomFilter;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
@@ -50,8 +51,17 @@ public class LogsQuery {
           final List<Address> addresses,
       @JsonDeserialize(using = TopicsDeserializer.class) @JsonProperty("topics")
           final List<List<LogTopic>> topics) {
-    this.addresses = addresses != null ? addresses : emptyList();
-    this.topics = topics != null ? topics : emptyList();
+    // Ordinarily this defensive copy wouldn't be surprising, style wise it should be an immutable
+    // collection. However, the semantics of the Ethereum JSON-RPC APIs ascribe meaning to a null
+    // value in lists for logs queries. We need to proactively put the values into a collection
+    // that won't throw a null pointer exception when checking to see if the list contains null.
+    // List.of(...) is one of the lists that reacts poorly to null member checks and is something
+    // that we should expect to see passed in. So we must copy into a null-tolerant list.
+    this.addresses = addresses != null ? new ArrayList<>(addresses) : emptyList();
+    this.topics =
+        topics != null
+            ? topics.stream().map(ArrayList::new).collect(Collectors.toList())
+            : emptyList();
     this.addressBlooms =
         this.addresses.stream()
             .map(address -> LogsBloomFilter.builder().insertBytes(address).build())

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/query/LogsQueryTest.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ */
+
+package org.hyperledger.besu.ethereum.api.query;
+
+import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.hyperledger.besu.ethereum.core.Address;
+import org.hyperledger.besu.ethereum.core.Log;
+import org.hyperledger.besu.ethereum.core.LogTopic;
+
+import java.util.List;
+
+import org.apache.tuweni.bytes.Bytes;
+import org.junit.Test;
+
+public class LogsQueryTest {
+
+  private static final Address FIRST_ADDRESS =
+      Address.fromHexString("8320fe7702b96808f7bbc0d4a888ed1468216cfd");
+  private static final LogTopic FIRST_ADDRESS_TOPIC =
+      LogTopic.fromHexString("0000000000000000000000008320fe7702b96808f7bbc0d4a888ed1468216cfd");
+  private static final LogTopic SECOND_ADDRESS_TOPIC =
+      LogTopic.fromHexString("0000000000000000000000009320fe7702b96808f7bbc0d4a888ed1468216cfd");
+  private static final LogTopic ERC20_TRANSFER_EVENT =
+      LogTopic.fromHexString("ddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef");
+
+  @Test
+  public void testMatches() {
+    final LogsQuery query =
+        new LogsQuery(
+            singletonList(FIRST_ADDRESS),
+            List.of(
+                singletonList(ERC20_TRANSFER_EVENT),
+                List.of(FIRST_ADDRESS_TOPIC, SECOND_ADDRESS_TOPIC)));
+
+    assertThat(query.matches(new Log(FIRST_ADDRESS, Bytes.EMPTY, List.of()))).isFalse();
+    assertThat(query.matches(new Log(FIRST_ADDRESS, Bytes.EMPTY, List.of(ERC20_TRANSFER_EVENT))))
+        .isFalse();
+    assertThat(
+            query.matches(
+                new Log(
+                    FIRST_ADDRESS,
+                    Bytes.EMPTY,
+                    List.of(ERC20_TRANSFER_EVENT, FIRST_ADDRESS_TOPIC))))
+        .isTrue();
+    assertThat(
+            query.matches(
+                new Log(
+                    FIRST_ADDRESS,
+                    Bytes.EMPTY,
+                    List.of(ERC20_TRANSFER_EVENT, SECOND_ADDRESS_TOPIC))))
+        .isTrue();
+    assertThat(
+            query.matches(
+                new Log(
+                    FIRST_ADDRESS,
+                    Bytes.EMPTY,
+                    List.of(ERC20_TRANSFER_EVENT, SECOND_ADDRESS_TOPIC, FIRST_ADDRESS_TOPIC))))
+        .isTrue();
+  }
+}