We are using WebSocketHttpHeaders for some of our web socket testing. I have noticed that using them does not work properly with the WebSocketStompClient

we do

WebSocketHttpHeaders headers= new WebSocketHttpHeaders();
        headers.add("Authorization", "Basic " + base64Credentials);
        StompSession stompSession = this.stompClient.connectAsync("ws://localhost:{port}/flowable-engage/engage-api/ws/conversation", headers, handler, this.serverPort)
            .get(10, TimeUnit.SECONDS);

This eventually calls the WebSocketClient#execute. In our test we use the SockJsClient, that one eventually calls InfoReceiver#executeInfoRequest, which does

HttpHeaders infoRequestHeaders = new HttpHeaders();
if (headers != null) {
    infoRequestHeaders.putAll(headers);
}

The problem is that the headers are WebSocketHttpHeaders, so doing putAll uses the field from the HttpHeaders and not from the WebSocketHttpHeaders

Comment From: sbrannen

Thanks for bringing this to our attention, @filiphr, just in time for 7.0 GA. 👍

It looks like WebSocketHeaders needs to override some of the methods introduced in conjunction with #33913 (see https://github.com/spring-projects/spring-framework/issues/33913#issuecomment-2560934534).

Comment From: filiphr

Thanks for looking into it for the 7.0 GA @sbrannen.

However, it's not about overriding the putAll in the WebSocketHeaders, it is already done. The problem is that a putAll is done on the HttpHeaders and WebSocketHeaders is passed as a parameter to the putAll. And the putAll is using the headers from the HttpHeaders.

I think that asSingleValueMap and asMultiValueMap should be overridden in the WebSocketHeaders and then in the HttpHeaders.putAll one of those methods should be used.

Or as an alternative the WebSocketHeaders should not use a local HttpHeaders.

I wanted to do a PR, but I was not sure which approach you would like to take, so I created an issue instead.

Comment From: sbrannen

Thanks for the feedback.

There are in fact a number of methods that need to be overridden. So I'll take care of those and address your use case as well.

Comment From: sbrannen

I think that asSingleValueMap and asMultiValueMap should be overridden in the WebSocketHeaders

Yes, both of those should be overridden in WebSocketHttpHeaders in any case.

and then in the HttpHeaders.putAll one of those methods should be used.

It turns out that the following is a simpler way to fix that issue.

    public void putAll(HttpHeaders headers) {
-       this.headers.putAll(headers.headers);
+       headers.forEach(this::put);
    }

The same is true for addAll(HttpHeaders) and putAll(Map).

Or as an alternative the WebSocketHeaders should not use a local HttpHeaders.

Indeed, that's a good idea, and I'll likely perform that in a separate issue.

Comment From: sbrannen

java WebSocketHttpHeaders headers = new WebSocketHttpHeaders(); headers.add("Authorization", "Basic " + base64Credentials);

Out of curiosity, are you aware of HttpHeaders.setBasicAuth(String) which does that for you? 😉

  • See #23204

Comment From: filiphr

Thanks a lot for the quick fix @sbrannen.

Out of curiosity, are you aware of HttpHeaders.setBasicAuth(String) which does that for you? 😉

I had forgotten about it. That particular test was written in May 2019, and I we haven't really touched it since. So that method was most likely not there and I do not remember if the other setBasicAuth methods were there. In any case, good point we should do some cleanup in that area