Don't encode ids array comma separators in getStateValidator request (#…

…8189)
Consensys · Apr 11, 2024 · c4b9a7a · c4b9a7a
1 parent 514539b
commit c4b9a7a
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 3 deletions.
diff --git a/...est/java/tech/pegasys/teku/validator/remote/typedef/OkHttpValidatorTypeDefClientTest.java b/...est/java/tech/pegasys/teku/validator/remote/typedef/OkHttpValidatorTypeDefClientTest.java
@@ -366,6 +366,21 @@ void postValidators_MakesExpectedRequest() throws Exception {
     assertThat(request.getBody().readUtf8()).isEqualTo("{\"ids\":[\"1\",\"0x1234\"]}");
   }
 
+  @TestTemplate
+  void getStateValidators_MakesExpectedRequest() throws Exception {
+    mockWebServer.enqueue(new MockResponse().setResponseCode(SC_NO_CONTENT));
+
+    okHttpValidatorTypeDefClient.getStateValidators(List.of("1", "0x1234"));
+
+    final RecordedRequest request = mockWebServer.takeRequest();
+    assertThat(request.getMethod()).isEqualTo("GET");
+
+    assertThat(request.getPath()).contains(ValidatorApiMethod.GET_VALIDATORS.getPath(emptyMap()));
+    // comma-separated GET query array parameters shouldn't be encoded
+    // and must pass AS IS as per RFC-3986
+    assertThat(request.getPath()).contains("?id=1,0x1234");
+  }
+
   @TestTemplate
   public void postValidators_WhenNoContent_ReturnsEmpty() {
     mockWebServer.enqueue(new MockResponse().setResponseCode(SC_NO_CONTENT));

diff --git a/...main/java/tech/pegasys/teku/validator/remote/typedef/handlers/AbstractTypeDefRequest.java b/...main/java/tech/pegasys/teku/validator/remote/typedef/handlers/AbstractTypeDefRequest.java
@@ -84,20 +84,29 @@ protected <T> Optional<T> get(
       final Map<String, String> urlParams,
       final Map<String, String> queryParams,
       final ResponseHandler<T> responseHandler) {
-    return get(apiMethod, urlParams, queryParams, Map.of(), responseHandler);
+    return get(apiMethod, urlParams, queryParams, emptyMap(), emptyMap(), responseHandler);
   }
 
   protected <T> Optional<T> get(
       final ValidatorApiMethod apiMethod,
       final Map<String, String> urlParams,
       final Map<String, String> queryParams,
+      final Map<String, String> encodedQueryParams,
       final Map<String, String> headers,
       final ResponseHandler<T> responseHandler) {
     final HttpUrl.Builder httpUrlBuilder = urlBuilder(apiMethod, urlParams);
     if (queryParams != null && !queryParams.isEmpty()) {
       queryParams.forEach(httpUrlBuilder::addQueryParameter);
     }
 
+    // The encodedQueryParams are considered to be encoded already
+    // and should not be encoded again. This is useful to prevent
+    // the comma in an array of values (e.g. id=1,2,3) from being
+    // encoded.
+    if (encodedQueryParams != null && !encodedQueryParams.isEmpty()) {
+      encodedQueryParams.forEach(httpUrlBuilder::addEncodedQueryParameter);
+    }
+
     final Request.Builder builder = requestBuilder().url(httpUrlBuilder.build());
     if (headers != null && !headers.isEmpty()) {
       headers.forEach(builder::addHeader);

diff --git a/...src/main/java/tech/pegasys/teku/validator/remote/typedef/handlers/CreateBlockRequest.java b/...src/main/java/tech/pegasys/teku/validator/remote/typedef/handlers/CreateBlockRequest.java
@@ -13,6 +13,7 @@
 
 package tech.pegasys.teku.validator.remote.typedef.handlers;
 
+import static java.util.Collections.emptyMap;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_NOT_FOUND;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_OK;
 import static tech.pegasys.teku.validator.remote.apiclient.ValidatorApiMethod.GET_UNSIGNED_BLINDED_BLOCK;
@@ -109,7 +110,13 @@ public Optional<BlockContainerAndMetaData> createUnsignedBlock(
       // application/octet-stream is preferred, but will accept application/json
       headers.put("Accept", "application/octet-stream;q=0.9, application/json;q=0.4");
     }
-    return get(apiMethod, Map.of("slot", slot.toString()), queryParams, headers, responseHandler)
+    return get(
+            apiMethod,
+            Map.of("slot", slot.toString()),
+            queryParams,
+            emptyMap(),
+            headers,
+            responseHandler)
         .map(
             response ->
                 new BlockContainerAndMetaData(

diff --git a/...n/java/tech/pegasys/teku/validator/remote/typedef/handlers/GetStateValidatorsRequest.java b/...n/java/tech/pegasys/teku/validator/remote/typedef/handlers/GetStateValidatorsRequest.java
@@ -13,6 +13,7 @@
 
 package tech.pegasys.teku.validator.remote.typedef.handlers;
 
+import static java.util.Collections.emptyMap;
 import static tech.pegasys.teku.ethereum.json.types.beacon.StateValidatorDataBuilder.STATE_VALIDATORS_RESPONSE_TYPE;
 import static tech.pegasys.teku.infrastructure.http.RestApiConstants.PARAM_ID;
 import static tech.pegasys.teku.validator.remote.apiclient.ValidatorApiMethod.GET_VALIDATORS;
@@ -35,8 +36,10 @@ public Optional<ObjectAndMetaData<List<StateValidatorData>>> getStateValidators(
       final List<String> validatorIds) {
     return get(
         GET_VALIDATORS,
-        Map.of(),
+        emptyMap(),
+        emptyMap(),
         Map.of(PARAM_ID, String.join(",", validatorIds)),
+        emptyMap(),
         new ResponseHandler<>(STATE_VALIDATORS_RESPONSE_TYPE));
   }
 }
diff --git a/...rc/main/java/tech/pegasys/teku/validator/remote/typedef/handlers/ProduceBlockRequest.java b/...rc/main/java/tech/pegasys/teku/validator/remote/typedef/handlers/ProduceBlockRequest.java
@@ -13,6 +13,7 @@
 
 package tech.pegasys.teku.validator.remote.typedef.handlers;
 
+import static java.util.Collections.emptyMap;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_NOT_FOUND;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_OK;
 import static tech.pegasys.teku.infrastructure.http.RestApiConstants.BUILDER_BOOST_FACTOR;
@@ -126,6 +127,7 @@ public Optional<BlockContainerAndMetaData> createUnsignedBlock(
             GET_UNSIGNED_BLOCK_V3,
             Map.of("slot", slot.toString()),
             queryParams,
+            emptyMap(),
             headers,
             this.responseHandler)
         .map(