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) { |
Administrator
|
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) { > |
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 <[hidden email]> 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) { > > |
Administrator
|
Good question, I guess most are OK, but what are Trailer Headers for instance?
BTW https://tomcat.apache.org/tomcat-9.0-doc/config/http2.html is our reference for the trunk Jacques Le 19/02/2019 à 10:24, Taher Alkhateeb a écrit : > 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 > <[hidden email]> 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) { >>> |
Administrator
|
I had a look, I don't think we need to worry about those vars.
If/When needed we can change defaults. Jacques Le 19/02/2019 à 19:05, Jacques Le Roux a écrit : > Good question, I guess most are OK, but what are Trailer Headers for instance? > > BTW https://tomcat.apache.org/tomcat-9.0-doc/config/http2.html is our reference for the trunk > > Jacques > > Le 19/02/2019 à 10:24, Taher Alkhateeb a écrit : >> 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 >> <[hidden email]> 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) { >>>> > |
Thank you Jacques and Taher for your review and useful comments: I have
committed this work with rev. 1854532. Taher, I have reviewed the configuration options but I don't feel like changing any of them by default; for now I have added a comment to the commit about them and I will leave the implementation of the ability to set them to a future enhancement. As a side note, this is a good example for a valid reason to provide the ability to run OFBiz in an external (rather than embedded) Tomcat instance (for which all the configuration options would be available outside of OFBiz): this would require to refactor various aspects of our framework, so I will postpone this discussion to another day :-) Regards, Jacopo On Mon, Feb 25, 2019 at 10:48 AM Jacques Le Roux < [hidden email]> wrote: > I had a look, I don't think we need to worry about those vars. > > If/When needed we can change defaults. > > Jacques > > Le 19/02/2019 à 19:05, Jacques Le Roux a écrit : > > Good question, I guess most are OK, but what are Trailer Headers for > instance? > > > > BTW https://tomcat.apache.org/tomcat-9.0-doc/config/http2.html is our > reference for the trunk > > > > Jacques > > > > Le 19/02/2019 à 10:24, Taher Alkhateeb a écrit : > >> 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 > >> <[hidden email]> 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) { > >>>> > > > |
Free forum by Nabble | Edit this page |