Add a test for empty response for ipV6 metadata and only read first c…

…haracter, not entire response.
larry-safran · Jan 27, 2024 · 7893544 · 7893544
1 parent 56821c7
commit 7893544
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 23 deletions.
diff --git a/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java b/googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java
@@ -160,7 +160,7 @@ private void resolve() {
       return;
     }
 
-    // must be xds
+    // Since not dns, we must be using xds
     if (executor == null) {
       executor = SharedResourceHolder.get(executorResource);
     }
@@ -283,13 +283,9 @@ private boolean queryIpv6SupportMetadata(String url) throws IOException {
         return false;
       }
       InputStream inputStream = con.getInputStream();
-      if (inputStream == null) {
-        return false;
-      }
-      try (Reader reader = new InputStreamReader(inputStream, Charsets.UTF_8)) {
-        String respBody = CharStreams.toString(reader);
-        return !respBody.isEmpty();
-      }
+      int c;
+      return (inputStream != null
+          && (c = inputStream.read()) != -1 && !Character.isWhitespace(c));
     } finally {
       if (con != null) {
         con.disconnect();

diff --git a/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java b/googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java
@@ -125,10 +125,10 @@ public void tearDown() {
   }
 
   private void createResolver() {
-    createResolver(true);
+    createResolver("1:1:1");
   }
 
-  private void createResolver(boolean supportIpV6) {
+  private void createResolver(String responseToIpV6) {
     HttpConnectionProvider httpConnections = new HttpConnectionProvider() {
       @Override
       public HttpURLConnection createConnection(String url) throws IOException {
@@ -139,9 +139,9 @@ public HttpURLConnection createConnection(String url) throws IOException {
               new ByteArrayInputStream(("/" + ZONE).getBytes(StandardCharsets.UTF_8)));
           return con;
         } else if (url.equals(GoogleCloudToProdNameResolver.METADATA_URL_SUPPORT_IPV6)) {
-          if (supportIpV6) {
+          if (responseToIpV6 != null) {
             when(con.getInputStream()).thenReturn(
-                new ByteArrayInputStream("2600:2d00:ffb0:0".getBytes(StandardCharsets.UTF_8)));
+                new ByteArrayInputStream(responseToIpV6.getBytes(StandardCharsets.UTF_8)));
           }
           return con;
         }
@@ -155,7 +155,7 @@ public HttpURLConnection createConnection(String url) throws IOException {
   }
 
   @Test
-  public void notOnGcpDelegateToDns() {
+  public void notOnGcp_DelegateToDns() {
     GoogleCloudToProdNameResolver.isOnGcp = false;
     createResolver();
     resolver.start(mockListener);
@@ -164,7 +164,7 @@ public void notOnGcpDelegateToDns() {
   }
 
   @Test
-  public void hasProvidedBootstrapDelegateToDns() {
+  public void hasProvidedBootstrap_DelegateToDns() {
     GoogleCloudToProdNameResolver.isOnGcp = true;
     GoogleCloudToProdNameResolver.xdsBootstrapProvided = true;
     GoogleCloudToProdNameResolver.enableFederation = false;
@@ -176,10 +176,10 @@ public void hasProvidedBootstrapDelegateToDns() {
 
   @SuppressWarnings("unchecked")
   @Test
-  public void onGcpAndNoProvidedBootstrap_DelegateToXds_noIpV6() {
+  public void onGcpAndNoProvidedBootstrap_DelegateToXds() {
     GoogleCloudToProdNameResolver.isOnGcp = true;
     GoogleCloudToProdNameResolver.xdsBootstrapProvided = false;
-    createResolver(false);
+    createResolver();
     resolver.start(mockListener);
     fakeExecutor.runDueTasks();
     assertThat(delegatedResolver.keySet()).containsExactly("xds");
@@ -188,7 +188,8 @@ public void onGcpAndNoProvidedBootstrap_DelegateToXds_noIpV6() {
     Map<String, ?> node = (Map<String, ?>) bootstrap.get("node");
     assertThat(node).containsExactly(
         "id", "C2P-991614323",
-        "locality", ImmutableMap.of("zone", ZONE));
+        "locality", ImmutableMap.of("zone", ZONE),
+        "metadata", ImmutableMap.of("TRAFFICDIRECTOR_DIRECTPATH_C2P_IPV6_CAPABLE", true));
     Map<String, ?> server = Iterables.getOnlyElement(
         (List<Map<String, ?>>) bootstrap.get("xds_servers"));
     assertThat(server).containsExactly(
@@ -203,10 +204,10 @@ public void onGcpAndNoProvidedBootstrap_DelegateToXds_noIpV6() {
 
   @SuppressWarnings("unchecked")
   @Test
-  public void onGcpAndNoProvidedBootstrapDelegateToXds() {
+  public void onGcpAndNoProvidedBootstrap_DelegateToXds_noIpV6() {
     GoogleCloudToProdNameResolver.isOnGcp = true;
     GoogleCloudToProdNameResolver.xdsBootstrapProvided = false;
-    createResolver();
+    createResolver(null);
     resolver.start(mockListener);
     fakeExecutor.runDueTasks();
     assertThat(delegatedResolver.keySet()).containsExactly("xds");
@@ -215,8 +216,7 @@ public void onGcpAndNoProvidedBootstrapDelegateToXds() {
     Map<String, ?> node = (Map<String, ?>) bootstrap.get("node");
     assertThat(node).containsExactly(
         "id", "C2P-991614323",
-        "locality", ImmutableMap.of("zone", ZONE),
-        "metadata", ImmutableMap.of("TRAFFICDIRECTOR_DIRECTPATH_C2P_IPV6_CAPABLE", true));
+        "locality", ImmutableMap.of("zone", ZONE));
     Map<String, ?> server = Iterables.getOnlyElement(
         (List<Map<String, ?>>) bootstrap.get("xds_servers"));
     assertThat(server).containsExactly(
@@ -229,9 +229,25 @@ public void onGcpAndNoProvidedBootstrapDelegateToXds() {
         ImmutableMap.of("xds_servers", ImmutableList.of(server)));
   }
 
+  @Test
+  public void emptyResolverMeetadataValue() {
+    GoogleCloudToProdNameResolver.isOnGcp = true;
+    GoogleCloudToProdNameResolver.xdsBootstrapProvided = false;
+    createResolver("");
+    resolver.start(mockListener);
+    fakeExecutor.runDueTasks();
+    assertThat(delegatedResolver.keySet()).containsExactly("xds");
+    verify(Iterables.getOnlyElement(delegatedResolver.values())).start(mockListener);
+    Map<String, ?> bootstrap = fakeBootstrapSetter.bootstrapRef.get();
+    Map<String, ?> node = (Map<String, ?>) bootstrap.get("node");
+    assertThat(node).containsExactly(
+        "id", "C2P-991614323",
+        "locality", ImmutableMap.of("zone", ZONE));
+  }
+
   @SuppressWarnings("unchecked")
   @Test
-  public void onGcpAndNoProvidedBootstrapAndFederationEnabledDelegateToXds() {
+  public void onGcpAndNoProvidedBootstrapAndFederationEnabled_DelegateToXds() {
     GoogleCloudToProdNameResolver.isOnGcp = true;
     GoogleCloudToProdNameResolver.xdsBootstrapProvided = false;
     GoogleCloudToProdNameResolver.enableFederation = true;
@@ -261,7 +277,7 @@ public void onGcpAndNoProvidedBootstrapAndFederationEnabledDelegateToXds() {
 
   @SuppressWarnings("unchecked")
   @Test
-  public void onGcpAndProvidedBootstrapAndFederationEnabledDontDelegateToXds() {
+  public void onGcpAndProvidedBootstrapAndFederationEnabled_DontDelegateToXds() {
     GoogleCloudToProdNameResolver.isOnGcp = true;
     GoogleCloudToProdNameResolver.xdsBootstrapProvided = true;
     GoogleCloudToProdNameResolver.enableFederation = true;