You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2016/03/08 11:11:12 UTC
svn commit: r1734044 - in /tomcat/trunk/java:
javax/servlet/http/PushBuilder.java
org/apache/catalina/core/ApplicationPushBuilder.java
org/apache/coyote/http2/Stream.java
org/apache/coyote/http2/StreamProcessor.java
Author: markt
Date: Tue Mar 8 10:11:12 2016
New Revision: 1734044
URL: http://svn.apache.org/viewvc?rev=1734044&view=rev
Log:
Based on EG discussion, add a boolean return value to push() so the application can tell if the push was sent or not.
Modified:
tomcat/trunk/java/javax/servlet/http/PushBuilder.java
tomcat/trunk/java/org/apache/catalina/core/ApplicationPushBuilder.java
tomcat/trunk/java/org/apache/coyote/http2/Stream.java
tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
Modified: tomcat/trunk/java/javax/servlet/http/PushBuilder.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/javax/servlet/http/PushBuilder.java?rev=1734044&r1=1734043&r2=1734044&view=diff
==============================================================================
--- tomcat/trunk/java/javax/servlet/http/PushBuilder.java (original)
+++ tomcat/trunk/java/javax/servlet/http/PushBuilder.java Tue Mar 8 10:11:12 2016
@@ -159,7 +159,8 @@ public interface PushBuilder {
PushBuilder lastModified(String lastModified);
/**
- * Generates the push request. After calling this method the following
+ * Generates the push request and sends it to the client unless pushes are
+ * not available for some reason. After calling this method the following
* fields are set to {@code null}:
* <ul>
* <li>{@code path}</li>
@@ -167,11 +168,14 @@ public interface PushBuilder {
* <li>{@code lastModified}</li>
* </ul>
*
+ * @return {@code true} if the push request was sent to the client,
+ * otherwise {@code false}
+ *
* @throws IllegalStateException If this method is called when {@code path}
* is {@code null}
* @throws IllegalArgumentException If the request to push requires a body
*/
- void push();
+ boolean push();
/**
* Obtain the name of the HTTP method that will be used for push requests
Modified: tomcat/trunk/java/org/apache/catalina/core/ApplicationPushBuilder.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/ApplicationPushBuilder.java?rev=1734044&r1=1734043&r2=1734044&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ApplicationPushBuilder.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/ApplicationPushBuilder.java Tue Mar 8 10:11:12 2016
@@ -38,6 +38,7 @@ import org.apache.catalina.Context;
import org.apache.catalina.connector.Request;
import org.apache.catalina.util.SessionConfig;
import org.apache.coyote.ActionCode;
+import org.apache.coyote.PushToken;
import org.apache.tomcat.util.buf.B2CConverter;
import org.apache.tomcat.util.buf.HexUtils;
import org.apache.tomcat.util.collections.CaseInsensitiveKeyMap;
@@ -322,7 +323,7 @@ public class ApplicationPushBuilder impl
@Override
- public void push() {
+ public boolean push() {
if (path == null) {
throw new IllegalStateException(sm.getString("pushBuilder.noPath"));
}
@@ -392,7 +393,8 @@ public class ApplicationPushBuilder impl
setHeader("cookie", generateCookieHeader(cookies,
catalinaRequest.getContext().getCookieProcessor()));
- coyoteRequest.action(ActionCode.PUSH_REQUEST, pushTarget);
+ PushToken pushToken = new PushToken(pushTarget);
+ coyoteRequest.action(ActionCode.PUSH_REQUEST, pushToken);
// Reset for next call to this method
pushTarget = null;
@@ -401,6 +403,8 @@ public class ApplicationPushBuilder impl
lastModified = null;
headers.remove("if-none-match");
headers.remove("if-modified-since");
+
+ return pushToken.getResult();
}
Modified: tomcat/trunk/java/org/apache/coyote/http2/Stream.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Stream.java?rev=1734044&r1=1734043&r2=1734044&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Stream.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Stream.java Tue Mar 8 10:11:12 2016
@@ -382,7 +382,10 @@ public class Stream extends AbstractStre
}
- void push(Request request) throws IOException {
+ boolean push(Request request) throws IOException {
+ if (!handler.getRemoteSettings().getEnablePush()) {
+ return false;
+ }
// Set the special HTTP/2 headers
request.getMimeHeaders().addValue(":method").duplicate(request.method());
request.getMimeHeaders().addValue(":scheme").duplicate(request.scheme());
@@ -404,6 +407,8 @@ public class Stream extends AbstractStre
}
push(handler, request, this);
+
+ return true;
}
Modified: tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java?rev=1734044&r1=1734043&r2=1734044&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java Tue Mar 8 10:11:12 2016
@@ -26,7 +26,7 @@ import org.apache.coyote.Adapter;
import org.apache.coyote.AsyncContextCallback;
import org.apache.coyote.ContainerThreadMarker;
import org.apache.coyote.ErrorState;
-import org.apache.coyote.Request;
+import org.apache.coyote.PushToken;
import org.apache.coyote.UpgradeToken;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
@@ -355,7 +355,8 @@ public class StreamProcessor extends Abs
// Servlet 4.0 Push requests
case PUSH_REQUEST: {
try {
- stream.push((Request) param);
+ PushToken pushToken = (PushToken) param;
+ pushToken.setResult(stream.push(pushToken.getPushTarget()));
} catch (IOException ioe) {
response.setErrorException(ioe);
setErrorState(ErrorState.CLOSE_CONNECTION_NOW, ioe);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1734044 - in /tomcat/trunk/java: javax/servlet/http/PushBuilder.java
org/apache/catalina/core/ApplicationPushBuilder.java org/apache/coyote/http2/Stream.java
org/apache/coyote/http2/StreamProcessor.java
Posted by Rémy Maucherat <re...@apache.org>.
2016-03-08 11:27 GMT+01:00 Mark Thomas <ma...@apache.org>:
> On 08/03/2016 10:20, Rémy Maucherat wrote:
> > 2016-03-08 11:11 GMT+01:00 <ma...@apache.org>:
> >
> >> Author: markt
> >> Date: Tue Mar 8 10:11:12 2016
> >> New Revision: 1734044
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1734044&view=rev
> >> Log:
> >> Based on EG discussion, add a boolean return value to push() so the
> >> application can tell if the push was sent or not.
> >>
> >
> > That was scary when some other guy suggested to throw an ISE. This stuff
> is
> > best effort, and that's it.
>
> I think the ISE idea originated with me but I agree it is bad idea
> because of the inherent race condition.
>
> Woops, sorry ;) I didn't remember it. But ultimately, push is not
necessary for the application to work, so I don't see how it could be an
error.
Rémy
Re: svn commit: r1734044 - in /tomcat/trunk/java:
javax/servlet/http/PushBuilder.java
org/apache/catalina/core/ApplicationPushBuilder.java
org/apache/coyote/http2/Stream.java
org/apache/coyote/http2/StreamProcessor.java
Posted by Mark Thomas <ma...@apache.org>.
On 08/03/2016 10:20, Rémy Maucherat wrote:
> 2016-03-08 11:11 GMT+01:00 <ma...@apache.org>:
>
>> Author: markt
>> Date: Tue Mar 8 10:11:12 2016
>> New Revision: 1734044
>>
>> URL: http://svn.apache.org/viewvc?rev=1734044&view=rev
>> Log:
>> Based on EG discussion, add a boolean return value to push() so the
>> application can tell if the push was sent or not.
>>
>
> That was scary when some other guy suggested to throw an ISE. This stuff is
> best effort, and that's it.
I think the ISE idea originated with me but I agree it is bad idea
because of the inherent race condition.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1734044 - in /tomcat/trunk/java: javax/servlet/http/PushBuilder.java
org/apache/catalina/core/ApplicationPushBuilder.java org/apache/coyote/http2/Stream.java
org/apache/coyote/http2/StreamProcessor.java
Posted by Rémy Maucherat <re...@apache.org>.
2016-03-08 11:11 GMT+01:00 <ma...@apache.org>:
> Author: markt
> Date: Tue Mar 8 10:11:12 2016
> New Revision: 1734044
>
> URL: http://svn.apache.org/viewvc?rev=1734044&view=rev
> Log:
> Based on EG discussion, add a boolean return value to push() so the
> application can tell if the push was sent or not.
>
That was scary when some other guy suggested to throw an ISE. This stuff is
best effort, and that's it.
Rémy
>
> Modified:
> tomcat/trunk/java/javax/servlet/http/PushBuilder.java
> tomcat/trunk/java/org/apache/catalina/core/ApplicationPushBuilder.java
> tomcat/trunk/java/org/apache/coyote/http2/Stream.java
> tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
>
> Modified: tomcat/trunk/java/javax/servlet/http/PushBuilder.java
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/javax/servlet/http/PushBuilder.java?rev=1734044&r1=1734043&r2=1734044&view=diff
>
> ==============================================================================
> --- tomcat/trunk/java/javax/servlet/http/PushBuilder.java (original)
> +++ tomcat/trunk/java/javax/servlet/http/PushBuilder.java Tue Mar 8
> 10:11:12 2016
> @@ -159,7 +159,8 @@ public interface PushBuilder {
> PushBuilder lastModified(String lastModified);
>
> /**
> - * Generates the push request. After calling this method the following
> + * Generates the push request and sends it to the client unless
> pushes are
> + * not available for some reason. After calling this method the
> following
> * fields are set to {@code null}:
> * <ul>
> * <li>{@code path}</li>
> @@ -167,11 +168,14 @@ public interface PushBuilder {
> * <li>{@code lastModified}</li>
> * </ul>
> *
> + * @return {@code true} if the push request was sent to the client,
> + * otherwise {@code false}
> + *
> * @throws IllegalStateException If this method is called when {@code
> path}
> * is {@code null}
> * @throws IllegalArgumentException If the request to push requires a
> body
> */
> - void push();
> + boolean push();
>
> /**
> * Obtain the name of the HTTP method that will be used for push
> requests
>
> Modified:
> tomcat/trunk/java/org/apache/catalina/core/ApplicationPushBuilder.java
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/ApplicationPushBuilder.java?rev=1734044&r1=1734043&r2=1734044&view=diff
>
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/core/ApplicationPushBuilder.java
> (original)
> +++ tomcat/trunk/java/org/apache/catalina/core/ApplicationPushBuilder.java
> Tue Mar 8 10:11:12 2016
> @@ -38,6 +38,7 @@ import org.apache.catalina.Context;
> import org.apache.catalina.connector.Request;
> import org.apache.catalina.util.SessionConfig;
> import org.apache.coyote.ActionCode;
> +import org.apache.coyote.PushToken;
> import org.apache.tomcat.util.buf.B2CConverter;
> import org.apache.tomcat.util.buf.HexUtils;
> import org.apache.tomcat.util.collections.CaseInsensitiveKeyMap;
> @@ -322,7 +323,7 @@ public class ApplicationPushBuilder impl
>
>
> @Override
> - public void push() {
> + public boolean push() {
> if (path == null) {
> throw new
> IllegalStateException(sm.getString("pushBuilder.noPath"));
> }
> @@ -392,7 +393,8 @@ public class ApplicationPushBuilder impl
> setHeader("cookie", generateCookieHeader(cookies,
> catalinaRequest.getContext().getCookieProcessor()));
>
> - coyoteRequest.action(ActionCode.PUSH_REQUEST, pushTarget);
> + PushToken pushToken = new PushToken(pushTarget);
> + coyoteRequest.action(ActionCode.PUSH_REQUEST, pushToken);
>
> // Reset for next call to this method
> pushTarget = null;
> @@ -401,6 +403,8 @@ public class ApplicationPushBuilder impl
> lastModified = null;
> headers.remove("if-none-match");
> headers.remove("if-modified-since");
> +
> + return pushToken.getResult();
> }
>
>
>
> Modified: tomcat/trunk/java/org/apache/coyote/http2/Stream.java
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Stream.java?rev=1734044&r1=1734043&r2=1734044&view=diff
>
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/http2/Stream.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/http2/Stream.java Tue Mar 8
> 10:11:12 2016
> @@ -382,7 +382,10 @@ public class Stream extends AbstractStre
> }
>
>
> - void push(Request request) throws IOException {
> + boolean push(Request request) throws IOException {
> + if (!handler.getRemoteSettings().getEnablePush()) {
> + return false;
> + }
> // Set the special HTTP/2 headers
>
> request.getMimeHeaders().addValue(":method").duplicate(request.method());
>
> request.getMimeHeaders().addValue(":scheme").duplicate(request.scheme());
> @@ -404,6 +407,8 @@ public class Stream extends AbstractStre
> }
>
> push(handler, request, this);
> +
> + return true;
> }
>
>
>
> Modified: tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java?rev=1734044&r1=1734043&r2=1734044&view=diff
>
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
> (original)
> +++ tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java Tue
> Mar 8 10:11:12 2016
> @@ -26,7 +26,7 @@ import org.apache.coyote.Adapter;
> import org.apache.coyote.AsyncContextCallback;
> import org.apache.coyote.ContainerThreadMarker;
> import org.apache.coyote.ErrorState;
> -import org.apache.coyote.Request;
> +import org.apache.coyote.PushToken;
> import org.apache.coyote.UpgradeToken;
> import org.apache.juli.logging.Log;
> import org.apache.juli.logging.LogFactory;
> @@ -355,7 +355,8 @@ public class StreamProcessor extends Abs
> // Servlet 4.0 Push requests
> case PUSH_REQUEST: {
> try {
> - stream.push((Request) param);
> + PushToken pushToken = (PushToken) param;
> +
> pushToken.setResult(stream.push(pushToken.getPushTarget()));
> } catch (IOException ioe) {
> response.setErrorException(ioe);
> setErrorState(ErrorState.CLOSE_CONNECTION_NOW, ioe);
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>