HttpHeaders should not leak pseudo headers in MultiMap API.

Author: vietj

vertx-core/src/main/java/io/vertx/core/http/impl/headers/HttpHeaders.java

@@ -18,12 +18,7 @@
 import io.vertx.core.http.impl.HttpUtils;
 import io.vertx.core.internal.http.HttpHeadersInternal;
 
-import java.util.AbstractList;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.TreeSet;
+import java.util.*;
 import java.util.stream.Collectors;
 
 /**
@@ -217,45 +212,72 @@ public HttpHeaders clear() {
 
   @Override
   public Iterator<Map.Entry<String, String>> iterator() {
-    Iterator<Map.Entry<CharSequence, CharSequence>> i = headers.iterator();
-    return new Iterator<Map.Entry<String, String>>() {
-      @Override
-      public boolean hasNext() {
-        return i.hasNext();
-      }
-      @Override
-      public Map.Entry<String, String> next() {
+    return new EntryIterator(headers.iterator());
+  }
+
+  private class EntryIterator implements Iterator<Map.Entry<String, String>> {
+
+    private Map.Entry<String, String> nextEntry(Iterator<Map.Entry<CharSequence, CharSequence>> i) {
+      while (i.hasNext()) {
         Map.Entry<CharSequence, CharSequence> next = i.next();
-        return new Map.Entry<String, String>() {
-          @Override
-          public String getKey() {
-            return next.getKey().toString();
-          }
-          @Override
-          public String getValue() {
-            return next.getValue().toString();
-          }
-          @Override
-          public String setValue(String value) {
-            if (!mutable) {
-              throw new IllegalStateException("Read only");
+        CharSequence name = next.getKey();
+        if (name.length() == 0 || name.charAt(0) != ':') {
+          return new Map.Entry<>() {
+            @Override
+            public String getKey() {
+              return next.getKey().toString();
+            }
+            @Override
+            public String getValue() {
+              return next.getValue().toString();
             }
-            String old = next.getValue().toString();
-            next.setValue(value);
-            return old;
-          }
-          @Override
-          public String toString() {
-            return next.toString();
-          }
-        };
+            @Override
+            public String setValue(String value) {
+              if (!mutable) {
+                throw new IllegalStateException("Read only");
+              }
+              String old = next.getValue().toString();
+              next.setValue(value);
+              return old;
+            }
+            @Override
+            public String toString() {
+              return next.toString();
+            }
+          };
+        }
       }
-    };
+      return null;
+    }
+
+    private final Iterator<Map.Entry<CharSequence, CharSequence>> iterator;
+    private Map.Entry<String, String> next;
+
+    public EntryIterator(Iterator<Map.Entry<CharSequence, CharSequence>> iterator) {
+      this.iterator = iterator;
+      this.next = nextEntry(iterator);
+    }
+
+    @Override
+    public boolean hasNext() {
+      return next != null;
+    }
+
+    @Override
+    public Map.Entry<String, String> next() {
+      Map.Entry<String, String> ret = next;
+      next = nextEntry(iterator);
+      return ret;
+    }
   }
 
   @Override
   public int size() {
-    return names().size();
+    int size = 0;
+    for (CharSequence name : headers.names()) {
+      size += isPseudoHeader(name) ? 0 : 1;
+    }
+    return size;
   }
 
   @Override
@@ -380,4 +402,8 @@ public HttpHeaders copy(boolean mutable) {
   HttpHeaders copy(boolean mutable, Headers<CharSequence, CharSequence, ?> headers) {
     return new HttpHeaders(mutable, new DefaultHttp2Headers().setAll(headers));
   }
+
+  private static boolean isPseudoHeader(CharSequence cs) {
+    return cs.length() > 0 && cs.charAt(0) == ':';
+  }
 }

vertx-core/src/test/java/io/vertx/tests/http/headers/Http2HeadersAdaptorsTest.java

@@ -11,20 +11,15 @@
 
 package io.vertx.tests.http.headers;
 
+import io.netty.handler.codec.Headers;
 import io.netty.handler.codec.http2.DefaultHttp2Headers;
-import io.vertx.core.MultiMap;
 import io.vertx.core.http.HttpMethod;
 import io.vertx.core.http.impl.headers.HttpHeaders;
 import io.vertx.core.http.impl.headers.HttpRequestHeaders;
-import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
 
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.stream.Collectors;
 
 import static org.junit.Assert.*;
@@ -34,84 +29,82 @@
  */
 public class Http2HeadersAdaptorsTest extends HeadersTest {
 
-  DefaultHttp2Headers headers;
-  MultiMap map;
-
-  @Before
-  public void setUp() {
-    headers = new DefaultHttp2Headers();
-    map = new HttpHeaders(headers);
-  }
-
   @Override
   protected HttpHeaders newMultiMap() {
     return new HttpHeaders(new DefaultHttp2Headers());
   }
 
   @Test
   public void testGetConvertUpperCase() {
-    map.set("foo", "foo_value");
-    assertEquals("foo_value", map.get("Foo"));
-    assertEquals("foo_value", map.get((CharSequence) "Foo"));
+    HttpHeaders headers = newMultiMap();
+    headers.set("foo", "foo_value");
+    assertEquals("foo_value", headers.get("Foo"));
+    assertEquals("foo_value", headers.get((CharSequence) "Foo"));
   }
 
   @Test
   public void testGetAllConvertUpperCase() {
-    map.set("foo", "foo_value");
-    assertEquals(Collections.singletonList("foo_value"), map.getAll("Foo"));
-    assertEquals(Collections.singletonList("foo_value"), map.getAll((CharSequence) "Foo"));
+    HttpHeaders headers = newMultiMap();
+    headers.set("foo", "foo_value");
+    assertEquals(Collections.singletonList("foo_value"), headers.getAll("Foo"));
+    assertEquals(Collections.singletonList("foo_value"), headers.getAll((CharSequence) "Foo"));
   }
 
   @Test
   public void testContainsConvertUpperCase() {
-    map.set("foo", "foo_value");
-    assertTrue(map.contains("Foo"));
-    assertTrue(map.contains((CharSequence) "Foo"));
+    HttpHeaders headers = newMultiMap();
+    headers.set("foo", "foo_value");
+    assertTrue(headers.contains("Foo"));
+    assertTrue(headers.contains((CharSequence) "Foo"));
   }
 
   @Test
   public void testSetConvertUpperCase() {
-    map.set("Foo", "foo_value");
-    map.set((CharSequence) "Bar", "bar_value");
-    map.set("Juu", (Iterable<String>)Collections.singletonList("juu_value"));
-    map.set("Daa", Collections.singletonList((CharSequence)"daa_value"));
-    assertHeaderNames("foo","bar", "juu", "daa");
+    HttpHeaders headers = newMultiMap();
+    headers.set("Foo", "foo_value");
+    headers.set((CharSequence) "Bar", "bar_value");
+    headers.set("Juu", (Iterable<String>)Collections.singletonList("juu_value"));
+    headers.set("Daa", Collections.singletonList((CharSequence)"daa_value"));
+    assertHeaderNames(headers, "foo","bar", "juu", "daa");
   }
 
   @Test
   public void testAddConvertUpperCase() {
-    map.add("Foo", "foo_value");
-    map.add((CharSequence) "Bar", "bar_value");
-    map.add("Juu", (Iterable<String>)Collections.singletonList("juu_value"));
-    map.add("Daa", Collections.singletonList((CharSequence)"daa_value"));
-    assertHeaderNames("foo","bar", "juu", "daa");
+    HttpHeaders headers = newMultiMap();
+    headers.add("Foo", "foo_value");
+    headers.add((CharSequence) "Bar", "bar_value");
+    headers.add("Juu", (Iterable<String>)Collections.singletonList("juu_value"));
+    headers.add("Daa", Collections.singletonList((CharSequence)"daa_value"));
+    assertHeaderNames(headers, "foo","bar", "juu", "daa");
   }
 
   @Test
   public void testRemoveConvertUpperCase() {
-    map.set("foo", "foo_value");
-    map.remove("Foo");
-    map.set("bar", "bar_value");
-    map.remove((CharSequence) "Bar");
-    assertHeaderNames();
+    HttpHeaders headers = newMultiMap();
+    headers.set("foo", "foo_value");
+    headers.remove("Foo");
+    headers.set("bar", "bar_value");
+    headers.remove((CharSequence) "Bar");
+    assertHeaderNames(headers);
   }
 
   @Ignore
   @Test
   public void testEntries() {
-    map.set("foo", Arrays.<String>asList("foo_value_1", "foo_value_2"));
-    List<Map.Entry<String, String>> entries = map.entries();
-    assertEquals(entries.size(), 1);
+    HttpHeaders headers = newMultiMap();
+    headers.set("foo", Arrays.<String>asList("foo_value_1", "foo_value_2"));
+    List<Map.Entry<String, String>> entries = headers.entries();
+    assertEquals(1, entries.size());
     assertEquals("foo", entries.get(0).getKey());
     assertEquals("foo_value_1", entries.get(0).getValue());
-    map.set("bar", "bar_value");
-    Map<String, String> collected = map.entries().stream().collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+    headers.set("bar", "bar_value");
+    Map<String, String> collected = headers.entries().stream().collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
     assertEquals("foo_value_1", collected.get("foo"));
     assertEquals("bar_value", collected.get("bar"));
   }
 
-  private void assertHeaderNames(String... expected) {
-    assertEquals(new HashSet<>(Arrays.asList(expected)), headers.names().stream().map(CharSequence::toString).collect(Collectors.toSet()));
+  private void assertHeaderNames(HttpHeaders headers,  String... expected) {
+    assertEquals(new HashSet<>(Arrays.asList(expected)), headers.unwrap().names().stream().map(CharSequence::toString).collect(Collectors.toSet()));
   }
 
   private HttpHeaders headers(HttpMethod method, String authority, String host) {
@@ -147,4 +140,18 @@ public void testAuthorityValidation() {
     assertFalse(headers(HttpMethod.CONNECT, "localhost:a", null).validate());
     assertFalse(headers(HttpMethod.CONNECT, null, "localhost:a").validate());
   }
+
+  @Test
+  public void testFilterPseudoHeaders() {
+    HttpHeaders headers = headers(HttpMethod.PUT, "localhost:8080", "another:4443");
+    List<String> names = new ArrayList<>();
+    headers.forEach(entry -> {
+      names.add(entry.getKey());
+    });
+    assertEquals(List.of("host"), names);
+    assertEquals(1, headers.size());
+    HttpHeaders other = newMultiMap();
+    other.setAll(headers);
+    assertEquals(1, other.unwrap().size());
+  }
 }