Enabling HTTP/2 in the embedded Tomcat connectors

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Enabling HTTP/2 in the embedded Tomcat connectors

Jacopo Cappellato-5
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) {
Reply | Threaded
Open this post in threaded view
|

Re: Enabling HTTP/2 in the embedded Tomcat connectors

Jacques Le Roux
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) {
>
Reply | Threaded
Open this post in threaded view
|

Re: Enabling HTTP/2 in the embedded Tomcat connectors

taher
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) {
> >
Reply | Threaded
Open this post in threaded view
|

Re: Enabling HTTP/2 in the embedded Tomcat connectors

Jacques Le Roux
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) {
>>>
Reply | Threaded
Open this post in threaded view
|

Re: Enabling HTTP/2 in the embedded Tomcat connectors

Jacques Le Roux
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) {
>>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: Enabling HTTP/2 in the embedded Tomcat connectors

Jacopo Cappellato-5
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) {
> >>>>
> >
>