Introduce a config option to use LAX ClientCookieEncoder instead of the default STRICT, close AsyncHttpClient#1416

Motivation:

Some users might not be ready to use the STRICT encoder if their server
requires invalid cookie values (which is a security issue).

Modification:

Introduce a config option to switch from STRICT to LAX encoder.

Result:

Easier migration path when using invalid cookies

Author: slandelle

client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java

@@ -193,6 +193,11 @@ public interface AsyncHttpClientConfig {
      */
     boolean isDisableUrlEncodingForBoundRequests();
 
+    /**
+     * @return true if AHC is to use a LAX cookie encoder, eg accept illegal chars in cookie value
+     */
+    boolean isUseLaxCookieEncoder();
+
     /**
      * In the case of a POST/Redirect/Get scenario where the server uses a 302 for the redirect, should AHC respond to the redirect with a GET or whatever the original method was.
      * Unless configured otherwise, for a 302, AHC, will use a GET for this case.

client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java

@@ -71,6 +71,7 @@ public class DefaultAsyncHttpClientConfig implements AsyncHttpClientConfig {
     private final Realm realm;
     private final int maxRequestRetry;
     private final boolean disableUrlEncodingForBoundRequests;
+    private final boolean useLaxCookieEncoder;
     private final boolean disableZeroCopy;
     private final boolean keepEncodingHeader;
     private final ProxyServerSelector proxyServerSelector;
@@ -146,6 +147,7 @@ private DefaultAsyncHttpClientConfig(//
             Realm realm,//
             int maxRequestRetry,//
             boolean disableUrlEncodingForBoundRequests,//
+            boolean useLaxCookieEncoder,//
             boolean disableZeroCopy,//
             boolean keepEncodingHeader,//
             ProxyServerSelector proxyServerSelector,//
@@ -222,6 +224,7 @@ private DefaultAsyncHttpClientConfig(//
         this.realm = realm;
         this.maxRequestRetry = maxRequestRetry;
         this.disableUrlEncodingForBoundRequests = disableUrlEncodingForBoundRequests;
+        this.useLaxCookieEncoder = useLaxCookieEncoder;
         this.disableZeroCopy = disableZeroCopy;
         this.keepEncodingHeader = keepEncodingHeader;
         this.proxyServerSelector = proxyServerSelector;
@@ -336,6 +339,11 @@ public boolean isDisableUrlEncodingForBoundRequests() {
         return disableUrlEncodingForBoundRequests;
     }
 
+    @Override
+    public boolean isUseLaxCookieEncoder() {
+        return useLaxCookieEncoder;
+    }
+
     @Override
     public boolean isDisableZeroCopy() {
         return disableZeroCopy;
@@ -627,6 +635,7 @@ public static class Builder {
         private Realm realm;
         private int maxRequestRetry = defaultMaxRequestRetry();
         private boolean disableUrlEncodingForBoundRequests = defaultDisableUrlEncodingForBoundRequests();
+        private boolean useLaxCookieEncoder = defaultUseLaxCookieEncoder();
         private boolean disableZeroCopy = defaultDisableZeroCopy();
         private boolean keepEncodingHeader = defaultKeepEncodingHeader();
         private ProxyServerSelector proxyServerSelector;
@@ -817,6 +826,11 @@ public Builder setDisableUrlEncodingForBoundRequests(boolean disableUrlEncodingF
             return this;
         }
 
+        public Builder setUseLaxCookieEncoder(boolean useLaxCookieEncoder) {
+            this.useLaxCookieEncoder = useLaxCookieEncoder;
+            return this;
+        }
+
         public Builder setDisableZeroCopy(boolean disableZeroCopy) {
             this.disableZeroCopy = disableZeroCopy;
             return this;
@@ -1147,6 +1161,7 @@ public DefaultAsyncHttpClientConfig build() {
                     realm, //
                     maxRequestRetry, //
                     disableUrlEncodingForBoundRequests, //
+                    useLaxCookieEncoder, //
                     disableZeroCopy, //
                     keepEncodingHeader, //
                     resolveProxyServerSelector(), //

client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java

@@ -111,6 +111,10 @@ public static boolean defaultDisableUrlEncodingForBoundRequests() {
         return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getBoolean(ASYNC_CLIENT_CONFIG_ROOT + "disableUrlEncodingForBoundRequests");
     }
 
+    public static boolean defaultUseLaxCookieEncoder() {
+        return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getBoolean(ASYNC_CLIENT_CONFIG_ROOT + "useLaxCookieEncoder");
+    }
+
     public static boolean defaultUseOpenSsl() {
         return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getBoolean(ASYNC_CLIENT_CONFIG_ROOT + "useOpenSsl");
     }

client/src/main/java/org/asynchttpclient/netty/request/NettyRequestFactory.java

@@ -57,9 +57,11 @@ public final class NettyRequestFactory {
     public static final String GZIP_DEFLATE = HttpHeaderValues.GZIP + "," + HttpHeaderValues.DEFLATE;
 
     private final AsyncHttpClientConfig config;
+    private final ClientCookieEncoder cookieEncoder;
 
     public NettyRequestFactory(AsyncHttpClientConfig config) {
         this.config = config;
+        cookieEncoder = config.isUseLaxCookieEncoder() ? ClientCookieEncoder.LAX : ClientCookieEncoder.STRICT;
     }
 
     private NettyBody body(Request request, boolean connect) {
@@ -107,7 +109,7 @@ private NettyBody body(Request request, boolean connect) {
                 nettyBody = new NettyInputStreamBody(inStreamGenerator.getInputStream(), inStreamGenerator.getContentLength());
 
             } else if (request.getBodyGenerator() instanceof ReactiveStreamsBodyGenerator) {
-                ReactiveStreamsBodyGenerator reactiveStreamsBodyGenerator = (ReactiveStreamsBodyGenerator)request.getBodyGenerator();
+                ReactiveStreamsBodyGenerator reactiveStreamsBodyGenerator = (ReactiveStreamsBodyGenerator) request.getBodyGenerator();
                 nettyBody = new NettyReactiveStreamsBody(reactiveStreamsBodyGenerator.getPublisher(), reactiveStreamsBodyGenerator.getContentLength());
 
             } else if (request.getBodyGenerator() != null) {
@@ -167,16 +169,17 @@ public NettyRequest newNettyRequest(Request request, boolean forceConnect, Proxy
             // assign headers as configured on request
             headers.set(request.getHeaders());
 
-            if (isNonEmpty(request.getCookies()))
-                headers.set(COOKIE, ClientCookieEncoder.STRICT.encode(request.getCookies()));
+            if (isNonEmpty(request.getCookies())) {
+                headers.set(COOKIE, cookieEncoder.encode(request.getCookies()));
+            }
 
             String userDefinedAcceptEncoding = headers.get(ACCEPT_ENCODING);
             if (userDefinedAcceptEncoding != null) {
                 // we don't support Brotly ATM
                 if (userDefinedAcceptEncoding.endsWith(BROTLY_ACCEPT_ENCODING_SUFFIX)) {
                     headers.set(ACCEPT_ENCODING, userDefinedAcceptEncoding.subSequence(0, userDefinedAcceptEncoding.length() - BROTLY_ACCEPT_ENCODING_SUFFIX.length()));
                 }
-                
+
             } else if (config.isCompressionEnforced()) {
                 headers.set(ACCEPT_ENCODING, GZIP_DEFLATE);
             }

client/src/main/resources/ahc-default.properties

@@ -21,6 +21,7 @@ org.asynchttpclient.strict302Handling=false
 org.asynchttpclient.keepAlive=true
 org.asynchttpclient.maxRequestRetry=5
 org.asynchttpclient.disableUrlEncodingForBoundRequests=false
+org.asynchttpclient.useLaxCookieEncoder=false
 org.asynchttpclient.removeQueryParamOnRedirect=true
 org.asynchttpclient.useOpenSsl=false
 org.asynchttpclient.useInsecureTrustManager=false