Repository: netty/netty
Issue: CorsConfigBuilder exposeHeaders and allowedRequestHeaders should accept CharSequence

### Expected behavior

In Netty 4.1, `HttpHeaderNames` constants are `AsciiString`s. It should be possible to directly pass those constants to `CorsConfigBuilder` methods such as `exposeHeaders`, `allowedRequestHeaders`.

### Actual behavior

Those methods take `String`, so one has to convert and call `toString` on the `AsciiString`s.

### Netty version

4.1.8.Final


Issue comments:
I can provide a PR.

The question is: "can we use CharSequence all the way down, meaning having CorsConfig::exposedHeaders and CorsConfig:: allowedRequestHeaders return `Set<CharSequence>` instead of `Set<String>`?".
> can we use CharSequence all the way down, meaning having CorsConfig::exposedHeaders and CorsConfig:: allowedRequestHeaders return Set<CharSequence> instead of Set<String>?

Answer my own question: no without breaking API (Java's Set is invariant).
[AsciiString#toString()](https://github.com/netty/netty/blob/4.1/common/src/main/java/io/netty/util/AsciiString.java#L1152) caches the string conversion. So with these static variables you should only have to pay the price 1 time. Also with the string improvements in jdk9 we may investigate removing `AsciiString` when we can support jdk9.
@Scottmitch Got it. Closing then, thanks!
Actually, in terms of API, we could just make the `CorsConfigBuilder` accept `CharSequence` and call `toString` internally. WDYT?
I haven't thought about it too much but the class is final so on the surface that seems reasonable.
I'll send a PR later tonight.
@slandelle cool.. Just ensure you also keep the old methods to preserve binary compatibility. 
@normanmaurer Well, I just plan to expand to super type in covariant position (`foo(String)` -> `foo(CharSequence)`), so it should be fine, shouldn't it?
Nope this is not fine see also:

https://github.com/netty/netty/issues/6283

> Am 09.02.2017 um 20:07 schrieb Stephane Landelle <notifications@github.com>:
> 
> @normanmaurer Well, I just plan to expand to super type in covariant position (foo(String) -> foo(CharSequence)), so it should be fine, shouldn't it?
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 

Mmm, #6283 looks very different to me (at least from a type system perspective).

`addAll(Future[])` (varargs are arrays) is NOT a generalization of `addAll(Promise[])`, event though `Promise` extends `Future` because arrays are invariant, not covariant.

My first guess was that it wasn't necessary in #6292 to restore `add(Promise)`.

Then, I'm not 100% sure of what happens in the Java bytecode, let me check.
My bad, Java arrays are covariant (they are invariant in Scala).
I stand corrected :)
PR: #6340

Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java
index 93d76ae82dc..3285581af77 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java
@@ -148,6 +148,38 @@ public CorsConfigBuilder exposeHeaders(final String... headers) {
         return this;
     }
 
+    /**
+     * Specifies the headers to be exposed to calling clients.
+     *
+     * During a simple CORS request, only certain response headers are made available by the
+     * browser, for example using:
+     * <pre>
+     * xhr.getResponseHeader(HttpHeaderNames.CONTENT_TYPE);
+     * </pre>
+     *
+     * The headers that are available by default are:
+     * <ul>
+     * <li>Cache-Control</li>
+     * <li>Content-Language</li>
+     * <li>Content-Type</li>
+     * <li>Expires</li>
+     * <li>Last-Modified</li>
+     * <li>Pragma</li>
+     * </ul>
+     *
+     * To expose other headers they need to be specified which is what this method enables by
+     * adding the headers to the CORS 'Access-Control-Expose-Headers' response header.
+     *
+     * @param headers the values to be added to the 'Access-Control-Expose-Headers' response header
+     * @return {@link CorsConfigBuilder} to support method chaining.
+     */
+    public CorsConfigBuilder exposeHeaders(final CharSequence... headers) {
+        for (CharSequence header: headers) {
+            exposeHeaders.add(header.toString());
+        }
+        return this;
+    }
+
     /**
      * By default cookies are not included in CORS requests, but this method will enable cookies to
      * be added to CORS requests. Calling this method will set the CORS 'Access-Control-Allow-Credentials'
@@ -215,6 +247,29 @@ public CorsConfigBuilder allowedRequestHeaders(final String... headers) {
         return this;
     }
 
+    /**
+     * Specifies the if headers that should be returned in the CORS 'Access-Control-Allow-Headers'
+     * response header.
+     *
+     * If a client specifies headers on the request, for example by calling:
+     * <pre>
+     * xhr.setRequestHeader('My-Custom-Header', "SomeValue");
+     * </pre>
+     * the server will receive the above header name in the 'Access-Control-Request-Headers' of the
+     * preflight request. The server will then decide if it allows this header to be sent for the
+     * real request (remember that a preflight is not the real request but a request asking the server
+     * if it allow a request).
+     *
+     * @param headers the headers to be added to the preflight 'Access-Control-Allow-Headers' response header.
+     * @return {@link CorsConfigBuilder} to support method chaining.
+     */
+    public CorsConfigBuilder allowedRequestHeaders(final CharSequence... headers) {
+        for (CharSequence header: headers) {
+            requestHeaders.add(header.toString());
+        }
+        return this;
+    }
+
     /**
      * Returns HTTP response headers that should be added to a CORS preflight response.
      *


Test patch: