Repository: netty/netty
Issue: Usage of DefaultHttpHeader.names() outcome may result in java.lang.ArrayStoreException
Labels: defect

`DefaultHttpHeader.names()` exposes HTTP header names as a `Set<String>`. Converting the resulting set to an array using `toArray(String[])` throws an exception: `java.lang.ArrayStoreException: io.netty.util.AsciiString`. 


### Expected behavior

I expect following code to return an array of String objects:

```
        Set<String> nettyHeaders = new DefaultHttpHeaders(true)
                .add(HttpHeaderNames.CONTENT_LENGTH, "50")
                .names();
        
        String[] namesArray = nettyHeaders.toArray(new String[nettyHeaders.size()]);
```

### Actual behavior

But the code above throws an exception:

```
java.lang.ArrayStoreException: io.netty.util.AsciiString

	at io.netty.handler.codec.HeadersUtils$DelegatingStringSet.fillArray(HeadersUtils.java:234)
	at io.netty.handler.codec.HeadersUtils$DelegatingStringSet.toArray(HeadersUtils.java:227)
	at io.netty.handler.codec.http.DefaultHttpHeadersTest.providesHeaderNamesAsArray(DefaultHttpHeadersTest.java:222)        ...
```
### Steps to reproduce

See below.

### Minimal yet complete reproducer code (or URL to code)

I have constructed this test to demonstrate the issue:

```
    @Test
    public void providesHeaderNamesAsArray() throws Exception {
        Set<String> nettyHeaders = new DefaultHttpHeaders(true)
                .add(HttpHeaderNames.CONTENT_LENGTH, "50")
                .names();

        String[] namesArray = nettyHeaders.toArray(new String[nettyHeaders.size()]);
        assertArrayEquals(namesArray, new String[]{"content-length"});
    }
```

### Root Cause

The problem lies in the DelegatingStringSet class in HeaderUtils.java, which adapts the underlying `CharSequence` to a `String`. Well, the adaptation is implemented for the `iterator()` only, but not for `toArray()` method. 


### Netty version

4.1.17 and latest Netty 4.1 branch.

### JVM version (e.g. `java -version`)

jdk 1.8.0_131

### OS version (e.g. `uname -a`)




Issue comments:
@mikkokar thanks! Will check
@mikkokar PTAL https://github.com/netty/netty/pull/7429 
Fixed via https://github.com/netty/netty/pull/7429

Pull request review comments:
i think the iterator says you need to call hasNext()?
@carl-mastrangelo yep this would be more correct... let me do this.
Why is this not passed in as a String[] ?
Good point. I guess its because the signature of `public <X> X[] toArray(X[] a)` is generic and it is still shared between both `toArray` methods. However we should be able to avoid forcing the `toString` conversion in the case of the `public Object[] toArray()` method. Also it the creation of a `Object[]` array here may not be correct ... we should consider doing something similar to `AbstractCollection`:

```java
T[] r = a.length >= size ? a :
                  (T[])java.lang.reflect.Array
                  .newInstance(a.getClass().getComponentType(), size);
```
@Scottmitch I disagree about not doing the string conversion. I think we must do it even if `Object[]` or `CharSequence[]` is passed in as we define it as `Set<String>` which should give the guarantee that only `String` will be stored in there. I will change how we create the Array tho.

Patch that resolved the issue:
diff --git a/codec/src/main/java/io/netty/handler/codec/HeadersUtils.java b/codec/src/main/java/io/netty/handler/codec/HeadersUtils.java
index d4062209b38..0582353b765 100644
--- a/codec/src/main/java/io/netty/handler/codec/HeadersUtils.java
+++ b/codec/src/main/java/io/netty/handler/codec/HeadersUtils.java
@@ -15,6 +15,7 @@
  */
 package io.netty.handler.codec;
 
+import java.util.AbstractCollection;
 import java.util.AbstractList;
 import java.util.Collection;
 import java.util.Iterator;
@@ -84,7 +85,7 @@ public static Set<String> namesAsString(Headers<CharSequence, CharSequence, ?> h
     private static final class StringEntryIterator implements Iterator<Entry<String, String>> {
         private final Iterator<Entry<CharSequence, CharSequence>> iter;
 
-        public StringEntryIterator(Iterator<Entry<CharSequence, CharSequence>> iter) {
+        StringEntryIterator(Iterator<Entry<CharSequence, CharSequence>> iter) {
             this.iter = iter;
         }
 
@@ -145,7 +146,7 @@ public String toString() {
     private static final class StringIterator<T> implements Iterator<String> {
         private final Iterator<T> iter;
 
-        public StringIterator(Iterator<T> iter) {
+        StringIterator(Iterator<T> iter) {
             this.iter = iter;
         }
 
@@ -167,7 +168,7 @@ public void remove() {
     }
 
     private static final class CharSequenceDelegatingStringSet extends DelegatingStringSet<CharSequence> {
-        public CharSequenceDelegatingStringSet(Set<CharSequence> allNames) {
+        CharSequenceDelegatingStringSet(Set<CharSequence> allNames) {
             super(allNames);
         }
 
@@ -182,10 +183,10 @@ public boolean addAll(Collection<? extends String> c) {
         }
     }
 
-    private abstract static class DelegatingStringSet<T> implements Set<String> {
+    private abstract static class DelegatingStringSet<T> extends AbstractCollection<String> implements Set<String> {
         protected final Set<T> allNames;
 
-        public DelegatingStringSet(Set<T> allNames) {
+        DelegatingStringSet(Set<T> allNames) {
             this.allNames = checkNotNull(allNames, "allNames");
         }
 
@@ -209,71 +210,11 @@ public Iterator<String> iterator() {
             return new StringIterator<T>(allNames.iterator());
         }
 
-        @Override
-        public Object[] toArray() {
-            Object[] arr = new Object[size()];
-            fillArray(arr);
-            return arr;
-        }
-
-        @SuppressWarnings("unchecked")
-        @Override
-        public <X> X[] toArray(X[] a) {
-            if (a == null || a.length < size()) {
-                X[] arr = (X[]) new Object[size()];
-                fillArray(arr);
-                return arr;
-            }
-            fillArray(a);
-            return a;
-        }
-
-        private void fillArray(Object[] arr) {
-            Iterator<T> itr = allNames.iterator();
-            for (int i = 0; i < size(); ++i) {
-                arr[i] = itr.next();
-            }
-        }
-
         @Override
         public boolean remove(Object o) {
             return allNames.remove(o);
         }
 
-        @Override
-        public boolean containsAll(Collection<?> c) {
-            for (Object o : c) {
-                if (!contains(o)) {
-                    return false;
-                }
-            }
-            return true;
-        }
-
-        @Override
-        public boolean removeAll(Collection<?> c) {
-            boolean modified = false;
-            for (Object o : c) {
-                if (remove(o)) {
-                    modified = true;
-                }
-            }
-            return modified;
-        }
-
-        @Override
-        public boolean retainAll(Collection<?> c) {
-            boolean modified = false;
-            Iterator<String> it = iterator();
-            while (it.hasNext()) {
-                if (!c.contains(it.next())) {
-                    it.remove();
-                    modified = true;
-                }
-            }
-            return modified;
-        }
-
         @Override
         public void clear() {
             allNames.clear();


Test patch:
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/DefaultHttpHeadersTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/DefaultHttpHeadersTest.java
index b09546eeb73..5514f23ff07 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/DefaultHttpHeadersTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/DefaultHttpHeadersTest.java
@@ -23,16 +23,13 @@
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Set;
 
 import static io.netty.handler.codec.http.HttpHeadersTestUtils.of;
 import static io.netty.util.AsciiString.contentEquals;
 import static java.util.Arrays.asList;
 import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 public class DefaultHttpHeadersTest {
     private static final CharSequence HEADER_NAME = "testHeader";
@@ -209,6 +206,16 @@ public void setObjectIterable() {
         assertDefaultValues(headers, HeaderValue.THREE);
     }
 
+    @Test
+    public void providesHeaderNamesAsArray() throws Exception {
+        Set<String> nettyHeaders = new DefaultHttpHeaders()
+                .add(HttpHeaderNames.CONTENT_LENGTH, 10)
+                .names();
+
+        String[] namesArray = nettyHeaders.toArray(new String[nettyHeaders.size()]);
+        assertArrayEquals(namesArray, new String[] { HttpHeaderNames.CONTENT_LENGTH.toString() });
+    }
+
     private static void assertDefaultValues(final DefaultHttpHeaders headers, final HeaderValue headerValue) {
         assertTrue(contentEquals(headerValue.asList().get(0), headers.get(HEADER_NAME)));
         List<CharSequence> expected = headerValue.asList();