Issue #4808 - Review HttpClient Request header APIs.

For some reason, Request.getHeaders() returned HttpFields,
but HttpRequest.getHeaders() returned HttpFields.Mutable,
and it was obviously wrong.

Fixed WebSocket code that was relying on this API error.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2020-07-13 16:44:07 +02:00
parent fc0f4b28f9
commit 7b05567b52
6 changed files with 32 additions and 31 deletions

View File

@ -372,7 +372,7 @@ public class HttpRequest implements Request
}
@Override
public HttpFields.Mutable getHeaders()
public HttpFields getHeaders()
{
return headers;
}

View File

@ -38,7 +38,6 @@ import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.HttpStatus;
@ -154,22 +153,26 @@ public abstract class ClientUpgradeRequest extends HttpRequest implements Respon
public void setSubProtocols(String... protocols)
{
HttpFields.Mutable headers = getHeaders();
headers(headers ->
{
headers.remove(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL);
for (String protocol : protocols)
{
headers.add(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL, protocol);
}
});
}
public void setSubProtocols(List<String> protocols)
{
HttpFields.Mutable headers = getHeaders();
headers(headers ->
{
headers.remove(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL);
for (String protocol : protocols)
{
headers.add(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL, protocol);
}
});
}
@Override
@ -282,7 +285,7 @@ public abstract class ClientUpgradeRequest extends HttpRequest implements Respon
.collect(Collectors.joining(","));
if (!StringUtil.isEmpty(extensionString))
getHeaders().add(HttpHeader.SEC_WEBSOCKET_EXTENSIONS, extensionString);
headers(headers -> headers.add(HttpHeader.SEC_WEBSOCKET_EXTENSIONS, extensionString));
// Notify the listener which may change the headers directly.
notifyUpgradeListeners((listener) -> listener.onHandshakeRequest(this));

View File

@ -413,13 +413,12 @@ public class WebSocketNegotiationTest extends WebSocketTester
upgradeRequest.setSubProtocols("test");
upgradeRequest.addExtensions("permessage-deflate;server_no_context_takeover");
CompletableFuture<String> extensionHeader = new CompletableFuture<>();
upgradeRequest.addListener(new UpgradeListener()
{
@Override
public void onHandshakeRequest(HttpRequest request)
{
request.getHeaders().put(HttpHeader.SEC_WEBSOCKET_EXTENSIONS, "permessage-deflate");
request.headers(headers -> headers.put(HttpHeader.SEC_WEBSOCKET_EXTENSIONS, "permessage-deflate"));
}
});

View File

@ -33,7 +33,7 @@ import org.eclipse.jetty.websocket.core.client.UpgradeListener;
public class JsrUpgradeListener implements UpgradeListener
{
private Configurator configurator;
private final Configurator configurator;
public JsrUpgradeListener(Configurator configurator)
{
@ -46,7 +46,7 @@ public class JsrUpgradeListener implements UpgradeListener
if (configurator == null)
return;
HttpFields.Mutable fields = request.getHeaders();
HttpFields fields = request.getHeaders();
Map<String, List<String>> originalHeaders = new HashMap<>();
fields.forEach(field ->
{
@ -59,8 +59,11 @@ public class JsrUpgradeListener implements UpgradeListener
configurator.beforeRequest(originalHeaders);
// Reset headers on HttpRequest per configurator
fields.clear();
originalHeaders.forEach(fields::put);
request.headers(headers ->
{
headers.clear();
originalHeaders.forEach(headers::put);
});
}
@Override

View File

@ -30,7 +30,6 @@ import java.util.concurrent.CountDownLatch;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
@ -72,12 +71,14 @@ public class NetworkFuzzer extends Fuzzer.Adapter implements Fuzzer, AutoCloseab
this.upgradeRequest = new RawUpgradeRequest(client, wsURI);
if (requestHeaders != null)
{
HttpFields.Mutable fields = this.upgradeRequest.getHeaders();
this.upgradeRequest.headers(fields ->
{
requestHeaders.forEach((name, value) ->
{
fields.remove(name);
fields.put(name, value);
});
});
}
this.client.start();
this.generator = new UnitGenerator(Behavior.CLIENT);

View File

@ -24,7 +24,6 @@ import java.util.List;
import java.util.stream.Collectors;
import org.eclipse.jetty.client.HttpResponse;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.io.EndPoint;
@ -49,18 +48,14 @@ public class JettyClientUpgradeRequest extends ClientUpgradeRequest
if (request != null)
{
// Copy request details into actual request
HttpFields.Mutable fields = getHeaders();
request.getHeaders().forEach(fields::put);
headers(fields -> request.getHeaders().forEach(fields::put));
// Copy manually created Cookies into place
List<HttpCookie> cookies = request.getCookies();
if (cookies != null)
{
// TODO: remove existing Cookie header (if set)?
for (HttpCookie cookie : cookies)
{
fields.add(HttpHeader.COOKIE, cookie.toString());
}
headers(fields -> cookies.forEach(cookie -> fields.add(HttpHeader.COOKIE, cookie.toString())));
}
// Copy sub-protocols