From 0049aa4322e9002b95cf3db0ed301a55204f7a29 Mon Sep 17 00:00:00 2001 From: Thomas Becker Date: Wed, 24 Jul 2013 10:13:53 +0200 Subject: [PATCH 1/2] 412418 HttpTransportOverSPDY fix race condition while sending push streams that could cause push data not to be sent. Fixes intermittent test issues in ReferrerPushStrategyTest --- .../server/http/HttpTransportOverSPDY.java | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/jetty-spdy/spdy-http-server/src/main/java/org/eclipse/jetty/spdy/server/http/HttpTransportOverSPDY.java b/jetty-spdy/spdy-http-server/src/main/java/org/eclipse/jetty/spdy/server/http/HttpTransportOverSPDY.java index 3ecc8b6a2f9..0c352e8dd39 100644 --- a/jetty-spdy/spdy-http-server/src/main/java/org/eclipse/jetty/spdy/server/http/HttpTransportOverSPDY.java +++ b/jetty-spdy/spdy-http-server/src/main/java/org/eclipse/jetty/spdy/server/http/HttpTransportOverSPDY.java @@ -293,15 +293,27 @@ public class HttpTransportOverSPDY implements HttpTransport private void sendNextResourceData() { - PushResource resource; - if(active.compareAndSet(false, true)) + LOG.debug("{} sendNextResourceData active: {}", hashCode(), active.get()); + if (active.compareAndSet(false, true)) { - resource = queue.poll(); - if (resource == null) + PushResource resource = queue.poll(); + if (resource != null) + { + LOG.debug("Opening new push channel for: {}", resource); + HttpChannelOverSPDY pushChannel = newHttpChannelOverSPDY(resource.getPushStream(), resource.getPushRequestHeaders()); + pushChannel.requestStart(resource.getPushRequestHeaders(), true); return; - LOG.debug("Opening new push channel for: {}", resource); - HttpChannelOverSPDY pushChannel = newHttpChannelOverSPDY(resource.getPushStream(), resource.getPushRequestHeaders()); - pushChannel.requestStart(resource.getPushRequestHeaders(), true); + } + + if (active.compareAndSet(true, false)) + { + if (queue.peek() != null) + sendNextResourceData(); + } + else + { + throw new IllegalStateException("active must not be false here! Concurrency bug!"); + } } } @@ -335,14 +347,15 @@ public class HttpTransportOverSPDY implements HttpTransport public void failed(Throwable x) { LOG.debug("Creating push stream failed.", x); + sendNextResourceData(); } }); } private void complete() { - if(!active.compareAndSet(true, false)) - LOG.warn("complete() called and active==false? That smells like a concurrency bug!", new IllegalStateException()); + if (!active.compareAndSet(true, false)) + throw new IllegalStateException(); sendNextResourceData(); } From 0f702624a3011e3e1a953a4f0df468537caccd0d Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 24 Jul 2013 10:33:02 +0200 Subject: [PATCH 2/2] 410668 - HTTP client should support the PATCH method. Modified the Request interface to add method(String) so that additional HTTP methods (such as from WebDAV) can be used. --- .../org/eclipse/jetty/client/HttpClient.java | 2 +- .../eclipse/jetty/client/HttpConnection.java | 6 ++--- .../eclipse/jetty/client/HttpReceiver.java | 2 +- .../org/eclipse/jetty/client/HttpRequest.java | 20 +++++++++++--- .../org/eclipse/jetty/client/HttpSender.java | 2 +- .../jetty/client/RedirectProtocolHandler.java | 11 ++++---- .../org/eclipse/jetty/client/api/Request.java | 15 ++++++++++- .../client/util/DigestAuthentication.java | 2 +- .../org/eclipse/jetty/http/HttpMethod.java | 26 +++++++++---------- 9 files changed, 57 insertions(+), 29 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 99a2fd7214f..3fe402825ed 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -387,7 +387,7 @@ public class HttpClient extends ContainerLifeCycle protected Request copyRequest(Request oldRequest, URI newURI) { Request newRequest = new HttpRequest(this, oldRequest.getConversationID(), newURI); - newRequest.method(oldRequest.getMethod()) + newRequest.method(oldRequest.method()) .version(oldRequest.getVersion()) .content(oldRequest.getContent()); for (HttpField header : oldRequest.getHeaders()) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java index 9685c20f76a..d578f1cc857 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java @@ -154,7 +154,7 @@ public class HttpConnection extends AbstractConnection implements Connection private void normalizeRequest(Request request) { - if (request.getMethod() == null) + if (request.method() == null) request.method(HttpMethod.GET); if (request.getVersion() == null) @@ -163,7 +163,7 @@ public class HttpConnection extends AbstractConnection implements Connection if (request.getIdleTimeout() <= 0) request.idleTimeout(client.getIdleTimeout(), TimeUnit.MILLISECONDS); - HttpMethod method = request.getMethod(); + String method = request.method(); HttpVersion version = request.getVersion(); HttpFields headers = request.getHeaders(); ContentProvider content = request.getContent(); @@ -178,7 +178,7 @@ public class HttpConnection extends AbstractConnection implements Connection path = "/"; request.path(path); } - if (destination.isProxied() && HttpMethod.CONNECT != method) + if (destination.isProxied() && !HttpMethod.CONNECT.is(method)) { path = request.getURI().toString(); request.path(path); diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java index 2f337313903..5a0e8959c7c 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java @@ -147,7 +147,7 @@ public class HttpReceiver implements HttpParser.ResponseHandler HttpConversation conversation = exchange.getConversation(); HttpResponse response = exchange.getResponse(); - parser.setHeadResponse(exchange.getRequest().getMethod() == HttpMethod.HEAD); + parser.setHeadResponse(HttpMethod.HEAD.is(exchange.getRequest().method())); response.version(version).status(status).reason(reason); // Probe the protocol handlers diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java index 73357bf9e0a..2e4cbec1c5c 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java @@ -29,6 +29,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.concurrent.ExecutionException; @@ -65,7 +66,7 @@ public class HttpRequest implements Request private String scheme; private String path; private String query; - private HttpMethod method; + private String method; private HttpVersion version; private long idleTimeout; private long timeout; @@ -126,6 +127,12 @@ public class HttpRequest implements Request @Override public HttpMethod getMethod() + { + return HttpMethod.fromString(method); + } + + @Override + public String method() { return method; } @@ -133,7 +140,14 @@ public class HttpRequest implements Request @Override public Request method(HttpMethod method) { - this.method = method; + this.method = method.asString(); + return this; + } + + @Override + public Request method(String method) + { + this.method = Objects.requireNonNull(method).toUpperCase(Locale.ENGLISH); return this; } @@ -581,6 +595,6 @@ public class HttpRequest implements Request @Override public String toString() { - return String.format("%s[%s %s %s]@%x", HttpRequest.class.getSimpleName(), getMethod(), getPath(), getVersion(), hashCode()); + return String.format("%s[%s %s %s]@%x", HttpRequest.class.getSimpleName(), method(), getPath(), getVersion(), hashCode()); } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java index 867022120bd..eff0576761c 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java @@ -176,7 +176,7 @@ public class HttpSender implements AsyncContentProvider.Listener String query = request.getQuery(); if (query != null) path += "?" + query; - requestInfo = new HttpGenerator.RequestInfo(request.getVersion(), request.getHeaders(), contentLength, request.getMethod().asString(), path); + requestInfo = new HttpGenerator.RequestInfo(request.getVersion(), request.getHeaders(), contentLength, request.method(), path); break; } case NEED_HEADER: diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java b/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java index ee177dca11f..7b9c42a7dd8 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java @@ -95,8 +95,9 @@ public class RedirectProtocolHandler extends Response.Listener.Empty implements { case 301: { - if (request.getMethod() == HttpMethod.GET || request.getMethod() == HttpMethod.HEAD) - redirect(result, request.getMethod(), newURI); + String method = request.method(); + if (HttpMethod.GET.is(method) || HttpMethod.HEAD.is(method)) + redirect(result, method, newURI); else fail(result, new HttpResponseException("HTTP protocol violation: received 301 for non GET or HEAD request", response)); break; @@ -105,13 +106,13 @@ public class RedirectProtocolHandler extends Response.Listener.Empty implements case 303: { // Redirect must be done using GET - redirect(result, HttpMethod.GET, newURI); + redirect(result, HttpMethod.GET.asString(), newURI); break; } case 307: { // Keep same method - redirect(result, request.getMethod(), newURI); + redirect(result, request.method(), newURI); break; } default: @@ -174,7 +175,7 @@ public class RedirectProtocolHandler extends Response.Listener.Empty implements } } - private void redirect(Result result, HttpMethod method, URI location) + private void redirect(Result result, String method, URI location) { final Request request = result.getRequest(); HttpConversation conversation = client.getConversation(request.getConversationID(), false); diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java index d582028272f..225137aa337 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java @@ -76,16 +76,29 @@ public interface Request int getPort(); /** - * @return the method of this request, such as GET or POST + * @return the method of this request, such as GET or POST, or null if the method is not a standard HTTP method + * @deprecated use {@link #method()} instead */ + @Deprecated HttpMethod getMethod(); + /** + * @return the method of this request, such as GET or POST, as a String + */ + String method(); + /** * @param method the method of this request, such as GET or POST * @return this request object */ Request method(HttpMethod method); + /** + * @param method the method of this request, such as GET or POST + * @return this request object + */ + Request method(String method); + /** * @return the path of this request, such as "/" or "/path" - without the query * @see #getQuery() diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/DigestAuthentication.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/DigestAuthentication.java index b6a54cb870d..55ac0ec5f51 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/DigestAuthentication.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/DigestAuthentication.java @@ -221,7 +221,7 @@ public class DigestAuthentication implements Authentication String A1 = user + ":" + realm + ":" + password; String hashA1 = toHexString(digester.digest(A1.getBytes(charset))); - String A2 = request.getMethod().asString() + ":" + request.getURI(); + String A2 = request.method() + ":" + request.getURI(); if ("auth-int".equals(qop)) A2 += ":" + toHexString(digester.digest(content)); String hashA2 = toHexString(digester.digest(A2.getBytes(charset))); diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java index 6b27173c7a8..15b20118745 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java @@ -41,7 +41,7 @@ public enum HttpMethod MOVE; /* ------------------------------------------------------------ */ - /** + /** * Optimised lookup to find a method name and trailing space in a byte array. * @param bytes Array containing ISO-8859-1 characters * @param position The first valid index @@ -70,22 +70,22 @@ public enum HttpMethod return HEAD; break; case 'O': - if (bytes[position+1]=='O' && bytes[position+2]=='T' && bytes[position+3]=='I' && length>=8 && + if (bytes[position+1]=='O' && bytes[position+2]=='T' && bytes[position+3]=='I' && length>=8 && bytes[position+4]=='O' && bytes[position+5]=='N' && bytes[position+6]=='S' && bytes[position+7]==' ' ) return OPTIONS; break; case 'D': - if (bytes[position+1]=='E' && bytes[position+2]=='L' && bytes[position+3]=='E' && length>=7 && + if (bytes[position+1]=='E' && bytes[position+2]=='L' && bytes[position+3]=='E' && length>=7 && bytes[position+4]=='T' && bytes[position+5]=='E' && bytes[position+6]==' ' ) return DELETE; break; case 'T': - if (bytes[position+1]=='R' && bytes[position+2]=='A' && bytes[position+3]=='C' && length>=6 && + if (bytes[position+1]=='R' && bytes[position+2]=='A' && bytes[position+3]=='C' && length>=6 && bytes[position+4]=='E' && bytes[position+5]==' ' ) return TRACE; break; case 'C': - if (bytes[position+1]=='O' && bytes[position+2]=='N' && bytes[position+3]=='N' && length>=8 && + if (bytes[position+1]=='O' && bytes[position+2]=='N' && bytes[position+3]=='N' && length>=8 && bytes[position+4]=='E' && bytes[position+5]=='C' && bytes[position+6]=='T' && bytes[position+7]==' ' ) return CONNECT; break; @@ -93,7 +93,7 @@ public enum HttpMethod if (bytes[position+1]=='O' && bytes[position+2]=='V' && bytes[position+3]=='E' && bytes[position+4]==' ') return MOVE; break; - + default: break; } @@ -101,7 +101,7 @@ public enum HttpMethod } /* ------------------------------------------------------------ */ - /** + /** * Optimised lookup to find a method name and trailing space in a byte array. * @param buffer buffer containing ISO-8859-1 characters * @return A HttpMethod if a match or null if no easy match. @@ -110,14 +110,14 @@ public enum HttpMethod { if (buffer.hasArray()) return lookAheadGet(buffer.array(),buffer.arrayOffset()+buffer.position(),buffer.arrayOffset()+buffer.limit()); - + // TODO use cache and check for space // return CACHE.getBest(buffer,0,buffer.remaining()); return null; } - + /* ------------------------------------------------------------ */ - public final static Trie CACHE= new ArrayTrie(); + public final static Trie CACHE= new ArrayTrie<>(); static { for (HttpMethod method : HttpMethod.values()) @@ -144,15 +144,15 @@ public enum HttpMethod /* ------------------------------------------------------------ */ public boolean is(String s) { - return toString().equalsIgnoreCase(s); + return toString().equalsIgnoreCase(s); } - + /* ------------------------------------------------------------ */ public ByteBuffer asBuffer() { return _buffer.asReadOnlyBuffer(); } - + /* ------------------------------------------------------------ */ public String asString() {