MockServer spuriously adds a second Set-Cookie header for certain cookies into the response returned for a forwarded request

[neil-hudson-SQC]

The Issue
We are using MockServer to forward a request and return the response from a target system. We see that the response returned from MockServer contains two Set-Cookie headers for one of the cookies set by the target system. This occurs for cookies where the first character of the cookie value is '!'. The request contains the 'correct' Set-Cookie header that originated with the target system and it contains a second Set-Cookie header where the value is the derived from the original one but with the leading '!' dropped.

This behaviour cause the interaction with the target system to fail when MockServer is placed between the client and the target system.

Sample
Set-Cookie headers returned when interaction is not via MockServer:

HTTP/1.1 200 OK
Cache-Control: no-cache, no-store
Pragma: no-cache
Expires: -1
x-hylandtypesflags: 3
COMPRESSION: 0
e: 1,0,1
n: 224,31,148,126,58,115,194,52,82,6,198,50,109,22,181,2,113,79,215,29,209,70,52,29,232,23,53,111,185,26,242,49,167,27,0,241,97,253,239,138,254,160,234,54,20,191,110,62,211,38,234,58,222,203,249,33,229,30,165,97,61,62,240,8,78,245,241,206,220,0,179,191,96,163,129,83,123,238,32,99,168,193,132,126,241,240,241,162,16,75,224,96,251,128,131,200,102,28,134,103,105,97,225,87,52,189,41,111,121,191,191,196,151,209,235,243,47,70,247,169,66,143,23,172,38,103,100,77,74,99,249,127,172,49,143,172,142,15,133,51,66,241,64,88,238,182,48,123,195,89,205,109,52,254,59,110,13,196,75,114,161,89,24,243,251,183,24,105,11,224,26,23,9,215,184,154,192,105,138,237,249,181,224,247,180,236,40,85,129,25,167,214,137,174,214,35,30,9,78,162,26,246,192,218,182,12,246,189,52,22,162,154,22,191,187,251,141,26,92,177,25,191,0,240,138,13,123,232,242,242,253,109,221,139,75,236,108,106,15,121,126,150,216,233,193,103,132,235,229,50,244,146,4,168,119,211
X-Robots-Tag: noindex
Strict-Transport-Security: max-age=31536000; includeSubDomains
Set-Cookie: ASP.NET_SessionId=sp5vpjs15tqndcth0kcahpyo; path=/; secure; HttpOnly; SameSite=Lax
Set-Cookie: FB_LB=!Yiprk9o9b/b/Ay5BcqRyIblXE4VujvL/mPRJLyVoJNFv6FtJtw4rXC4XRXxi0xEEDIWuHEs1V1Kdrw==; path=/; Httponly; Secure; SameSite=none
X-UA-Compatible: IE=11
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
Date: Sun, 26 May 2024 17:46:52 GMT
connection: keep-alive
content-length: 0

Set-Cookie headers returned when interaction is via MockServer:

HTTP/1.1 200 OK
Cache-Control: no-cache, no-store
Pragma: no-cache
Expires: -1
x-hylandtypesflags: 3
COMPRESSION: 0
e: 1,0,1
n: 224,31,148,126,58,115,194,52,82,6,198,50,109,22,181,2,113,79,215,29,209,70,52,29,232,23,53,111,185,26,242,49,167,27,0,241,97,253,239,138,254,160,234,54,20,191,110,62,211,38,234,58,222,203,249,33,229,30,165,97,61,62,240,8,78,245,241,206,220,0,179,191,96,163,129,83,123,238,32,99,168,193,132,126,241,240,241,162,16,75,224,96,251,128,131,200,102,28,134,103,105,97,225,87,52,189,41,111,121,191,191,196,151,209,235,243,47,70,247,169,66,143,23,172,38,103,100,77,74,99,249,127,172,49,143,172,142,15,133,51,66,241,64,88,238,182,48,123,195,89,205,109,52,254,59,110,13,196,75,114,161,89,24,243,251,183,24,105,11,224,26,23,9,215,184,154,192,105,138,237,249,181,224,247,180,236,40,85,129,25,167,214,137,174,214,35,30,9,78,162,26,246,192,218,182,12,246,189,52,22,162,154,22,191,187,251,141,26,92,177,25,191,0,240,138,13,123,232,242,242,253,109,221,139,75,236,108,106,15,121,126,150,216,233,193,103,132,235,229,50,244,146,4,168,119,211
X-Robots-Tag: noindex
Strict-Transport-Security: max-age=31536000; includeSubDomains
Set-Cookie: ASP.NET_SessionId=ps1dj25k1m3blpnsufinhn0z; path=/; secure; HttpOnly; SameSite=Lax
Set-Cookie: FB_LB=!fn6HDZICizTBnZ9BcqRyIblXE4VujhdY+z9pcYkPNZydJj6YhxP1P6AqkUks4BIFl6XrbzoqCoELAg==; path=/; Httponly; Secure; SameSite=none
X-UA-Compatible: IE=11
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
Date: Mon, 27 May 2024 04:51:14 GMT
connection: keep-alive
content-length: 0
set-cookie: FB_LB=fn6HDZICizTBnZ9BcqRyIblXE4VujhdY+z9pcYkPNZydJj6YhxP1P6AqkUks4BIFl6XrbzoqCoELAg==

Note the additional. derived 'set-cookie' header at the end of the response headers. Note the value has the leading '!' dropped.

The Cause
The source of the problem has been tracked down to the method shown below:

public class MockServerHttpResponseToFullHttpResponse {

    // Other methods removed for clarity

    private void setCookies(HttpResponse httpResponse, DefaultHttpResponse response) {
        if (httpResponse.getCookieMap() != null) {
            for (Map.Entry<NottableString, NottableString> cookie : httpResponse.getCookieMap().entrySet()) {
                if (httpResponse.cookieHeaderDoesNotAlreadyExists(cookie.getKey().getValue(), cookie.getValue().getValue())) {
                    response.headers().add(SET_COOKIE, io.netty.handler.codec.http.cookie.ServerCookieEncoder.LAX.encode(new DefaultCookie(cookie.getKey().getValue(), cookie.getValue().getValue())));
                }
            }
        }
    }
}

Its aim is to detect where a cookie is (a) in the cookie map of httpResponse and (b) not present in the cookie header of the httpResponse or has a different value set in the header and, when this is detected, add the cookie into the cookie header of the the httpResponse.
This check malfunctions when the cookie value starts with a '!', it fails to recognise that the cookie is already there and adds the cookie again but with a value that drops the leading '!'.
The cause of the malfunction is that the candidate cookie value is held in the type:

public class NottableString extends ObjectWithJsonToString implements Comparable<NottableString> {

    public static final char NOT_CHAR = '!';
    private static final String EMPTY_STRING = "";

which treats leading '!' characters as having a specific 'NOT' meaning. There are three elements that represent the 'value' in this type:

    private final String value;
    private final boolean isBlank;
    private final Boolean not;

In the case of strings starting with a '!' the Boolean not is set to true and the String value contains a string that has the '!' removed from the original string value.
The method to ensure all cookies are in the header with the correct value uses the following expression for the cookie value when checking for the cookie already being there and to add it if it is found to be missing.

cookie.getValue().getValue()

This returns the String value from the NottableString and so where the original value starts with a '!' returns a value without the leading '!'. This cause the method to miss that the set-cookie pair is already there and to add it again but with the corrupted value.

Possible Fix
A possible fix is to replace:

cookie.getValue().getValue()

with the method:

cookie.getValue().toString()

that does retain the leading '!'. This would give:

public class MockServerHttpResponseToFullHttpResponse {

	// Other methods removed for clarity

    private void setCookies(HttpResponse httpResponse, DefaultHttpResponse response) {
        if (httpResponse.getCookieMap() != null) {
            for (Map.Entry<NottableString, NottableString> cookie : httpResponse.getCookieMap().entrySet()) {
                if (httpResponse.cookieHeaderDoesNotAlreadyExists(cookie.getKey().getValue(), cookie.getValue().toString())) {
                    response.headers().add(SET_COOKIE, io.netty.handler.codec.http.cookie.ServerCookieEncoder.LAX.encode(new DefaultCookie(cookie.getKey().getValue(), cookie.getValue().toString())));
                }
            }
        }
    }
}

We have trialled this change and it does remove the duplication.

MockServer version
This issue has been observed in:

• Docker Image containing 5.14.x
• A local build based on the 5.15.x master branch cloned on 2024-05-26

To Reproduce
An expectation that exhibits the issue would be of the form:

    public static void main( String[] args )
    {
        var vMsc = new MockServerClient("localhost", 1090);
        var v = vMsc.reset();

        vMsc
          .when(
            request()
          )
          .forward(
            forwardOverriddenRequest(
              request().
                withSocketAddress(HYLAND,443, SocketAddress.Scheme.HTTPS).replaceHeader(header("Host",HYLAND)).withKeepAlive(false)
            )
          );
      var vInputManager = new Scanner(System.in);
      vInputManager.nextLine();
    }

The system needs to return a Set-Cookie header that sets a value starting with a '!' an example being:

Set-Cookie: FB_LB=!Yiprk9o9b/b/Ay5BcqRyIblXE4VujvL/mPRJLyVoJNFv6FtJtw4rXC4XRXxi0xEEDIWuHEs1V1Kdrw==; path=/; Httponly; ``` 

Expected behaviour
Cookies with values starting with '!' that are already in the header should not cause a second Set-Cookie header to be added.
Cookies with values starting with '!' that are missing in the header should cause a Set-Cookie header to be added with the value retaining the leading '!', they should not be set to a value that drops the leading '!'.