clean and simple implementation, +1 on a side note, I wonder if we need to set any of the http2 attributes listed in [1] or whether the defaults are okay.
[1] https://tomcat.apache.org/tomcat-8.5-doc/config/http2.html On Mon, Feb 18, 2019 at 5:58 PM Jacques Le Roux <jacques.le.r...@les7arts.com> wrote: > > Hi Jacopo, > > Sounds good to me, we can always easily revert in case of unexpected issue > anyway > > Thanks > > Jacques > > Le 18/02/2019 à 11:43, Jacopo Cappellato a écrit : > > Hi all, > > > > I think it is time to enable the instance of Tomcat that is embedded in > > OFBiz to communicate using the HTTP/2 protocol, when the client supports it. > > For your review, before I commit, I am pasting here the patch that will > > enable it (it is quite simple) . > > In it I have enabled HTTP/2 by default, by setting upgradeProtocol=true, in > > the http and https connectors (but they will continue to support also > > HTTP/1.1); if the new property "upgradeProtocol", that I have introduced > > for this specific purpose, is not set (as it would be the case in custom > > configuration files) then the new protocol will not be enabled. Does the > > approach look good to you? > > > > Thanks, > > > > Jacopo > > > > PS: you can test it, for example, using curl: > > > > curl -vso /dev/null --http2 http://localhost:8080 > > > > > > Index: framework/catalina/ofbiz-component.xml > > =================================================================== > > --- framework/catalina/ofbiz-component.xml (revision 1853787) > > +++ framework/catalina/ofbiz-component.xml (working copy) > > @@ -99,6 +99,7 @@ > > <!--<property name="address" value=""/>--> > > <property name="port" value="8080"/> > > <property name="protocol" value="HTTP/1.1"/> > > + <property name="upgradeProtocol" value="true"/> > > <property name="scheme" value="http"/> > > <property name="secure" value="false"/> > > <property name="URIEncoding" value="UTF-8"/> > > @@ -128,6 +129,7 @@ > > <!--<property name="address" value=""/>--> > > <property name="port" value="8443"/> > > <property name="protocol" value="HTTP/1.1"/> > > + <property name="upgradeProtocol" value="true"/> > > <property name="scheme" value="https"/> > > <property name="secure" value="true"/> > > <property name="SSLEnabled" value="true"/> > > @@ -183,6 +185,7 @@ > > <!--<property name="address" value=""/>--> > > <property name="port" value="8080"/> > > <property name="protocol" value="HTTP/1.1"/> > > + <property name="upgradeProtocol" value="true"/> > > <property name="scheme" value="http"/> > > <property name="secure" value="false"/> > > <property name="URIEncoding" value="UTF-8"/> > > @@ -194,6 +197,7 @@ > > <!--<property name="address" value=""/>--> > > <property name="port" value="8443"/> > > <property name="protocol" value="HTTP/1.1"/> > > + <property name="upgradeProtocol" value="true"/> > > <property name="scheme" value="https"/> > > <property name="secure" value="true"/> > > <property name="SSLEnabled" value="true"/> > > Index: > > framework/catalina/src/main/java/org/apache/ofbiz/catalina/container/CatalinaContainer.java > > =================================================================== > > --- > > framework/catalina/src/main/java/org/apache/ofbiz/catalina/container/CatalinaContainer.java > > (revision > > 1853787) > > +++ > > framework/catalina/src/main/java/org/apache/ofbiz/catalina/container/CatalinaContainer.java > > (working > > copy) > > @@ -63,6 +63,7 @@ > > import org.apache.catalina.util.ServerInfo; > > import org.apache.catalina.valves.AccessLogValve; > > import org.apache.catalina.webresources.StandardRoot; > > +import org.apache.coyote.http2.Http2Protocol; > > import org.apache.ofbiz.base.component.ComponentConfig; > > import org.apache.ofbiz.base.concurrent.ExecutionPool; > > import org.apache.ofbiz.base.container.Container; > > @@ -417,9 +418,12 @@ > > private Connector prepareConnector(Property connectorProp) { > > Connector connector = new > > Connector(ContainerConfig.getPropertyValue(connectorProp, "protocol", > > "HTTP/1.1")); > > connector.setPort(ContainerConfig.getPropertyValue(connectorProp, > > "port", 0) + Start.getInstance().getConfig().portOffset); > > - > > + if ("true".equals(ContainerConfig.getPropertyValue(connectorProp, > > "upgradeProtocol", "false"))) { > > + connector.addUpgradeProtocol(new Http2Protocol()); > > + Debug.logInfo("Tomcat " + connector + ": enabled HTTP/2", > > module); > > + } > > connectorProp.properties.values().stream() > > - .filter(prop -> !"protocol".equals(prop.name) && > > !"port".equals(prop.name)) > > + .filter(prop -> !"protocol".equals(prop.name) && > > !"upgradeProtocol".equals(prop.name) && !"port".equals(prop.name)) > > .forEach(prop -> { > > if (IntrospectionUtils.setProperty(connector, prop.name, > > prop.value)) { > > if (prop.name.indexOf("Pass") != -1) { > >
