You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Keiichi Fujino <kf...@apache.org> on 2011/06/07 13:37:33 UTC

[PROPOSAL] Enable session replication by default.

Hi All.

Tomcat7.0.15 can not replicate session if web application defines
<distributable/> in WEB-INF/web.xml.
Tomcat7.0.14 can replicate session if web application defines
<distributable/> in WEB-INF/web.xml.
The behavior of this default was changed by rev1130618.
The cause is conf/web.xml is not distributable.

To enable session replication by default,
I thought two plans.

PlanA:
Add WebXml#setDistributable(true) to ContextConfig#webConfig().

Index: java/org/apache/catalina/startup/ContextConfig.java
===================================================================
--- java/org/apache/catalina/startup/ContextConfig.java	(revision 1132530)
+++ java/org/apache/catalina/startup/ContextConfig.java	(working copy)
@@ -1195,6 +1195,7 @@
          */
         WebXml webXmlDefaultFragment = createWebXml();
         webXmlDefaultFragment.setOverridable(true);
+        webXmlDefaultFragment.setDistributable(true);

         // Parse global web.xml if present
         InputSource globalWebXml = getGlobalWebXmlSource();


PlanB:
Add <distributable/> to conf/web.xml

Index: conf/web.xml
===================================================================
--- conf/web.xml	(revision 1127122)
+++ conf/web.xml	(working copy)
@@ -4176,4 +4176,6 @@
         <welcome-file>index.jsp</welcome-file>
     </welcome-file-list>

+  <!-- ====================  distributable ===================== -->
+    <distributable/>
 </web-app>


I intend to commit planA.
Any objections or other ideas?

-- 
Keiichi.Fujino

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PROPOSAL] Enable session replication by default.

Posted by Mark Thomas <ma...@apache.org>.
On 07/06/2011 22:42, Konstantin Kolinko wrote:
> BTW, two minor thoughts wrt. o.a.c.startup.ContextConfig#webConfig()
> 1) Line 1208
> 
>         // Parse host level web.xml if present
>         // Additive apart from welcome pages
>         webXml.setReplaceWelcomeFiles(true);
>         InputSource hostWebXml = getHostWebXmlSource();
>         parseWebXml(hostWebXml, webXmlDefaultFragment, false);
> 
> The "webXml.setReplaceWelcomeFiles(true);" call is needed,

I don't think it is since webXml is now empty when the main web.xml is
parsed into it.

> but from the comment it seems to me that
> webXmlDefaultFragment.setReplaceWelcomeFiles(true); is also needed
> here,
> because webXmlDefaultFragment is the argument to parseWebXml() call
> and not webXml.

Yep, that does look to be required. Hmm. Looks like it is no longer
possible to override the welcome files in an app's web.xml. That'll need
fixing too.

> 2)
> Line 1223:
> All of the code below
>         // Assuming 0 is safe for what is required in this case
>         double webXmlVersion = 0;
>         if (webXml.getVersion() != null) {
>             webXmlVersion = Double.parseDouble(webXml.getVersion());
>         }
> 
>         if (webXmlVersion >= 3) {
>  Just can be replaced with one line
>         if (webXml.getMajorVersion() >= 3) {
> 
> Note, that webXml.getVersion() never returns null.

Fixed. Thanks.

Mark



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PROPOSAL] Enable session replication by default.

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/6/8 Mark Thomas <ma...@apache.org>:
> On 07/06/2011 21:54, Rainer Jung wrote:
>> On 07.06.2011 21:39, Mark Thomas wrote:
>>> On 07/06/2011 20:24, Jess Holle wrote:
>>>> web apps whose web.xml does not specify distributable should certainly
>>>> not be treated as such -- that would be a spec violation and break lots
>>>> of web apps.
>>>
>>> How many times do I have to write this? This is NOT what is being
>>> proposed. All the proposed change does is fix a regression that prevents
>>> *any* web application for declaring itself as distributable.
>>
>> +1 to that change and to 7.0.15 is broken.
>
> I've confirmed with testing that the proposed fix does exactly what is
> intended and fixes the regression. I'll be committing it shortly.
>

I reviewed it and am revoking my veto wrt. Plan A.
+1 to the change to ContextConfig.java,

BTW, two minor thoughts wrt. o.a.c.startup.ContextConfig#webConfig()
1) Line 1208

        // Parse host level web.xml if present
        // Additive apart from welcome pages
        webXml.setReplaceWelcomeFiles(true);
        InputSource hostWebXml = getHostWebXmlSource();
        parseWebXml(hostWebXml, webXmlDefaultFragment, false);

The "webXml.setReplaceWelcomeFiles(true);" call is needed, but from
the comment it seems to me that
webXmlDefaultFragment.setReplaceWelcomeFiles(true); is also needed
here,
because webXmlDefaultFragment is the argument to parseWebXml() call
and not webXml.

2)
Line 1223:
All of the code below
        // Assuming 0 is safe for what is required in this case
        double webXmlVersion = 0;
        if (webXml.getVersion() != null) {
            webXmlVersion = Double.parseDouble(webXml.getVersion());
        }

        if (webXmlVersion >= 3) {
 Just can be replaced with one line
        if (webXml.getMajorVersion() >= 3) {

Note, that webXml.getVersion() never returns null.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PROPOSAL] Enable session replication by default.

Posted by Mark Thomas <ma...@apache.org>.
On 07/06/2011 21:54, Rainer Jung wrote:
> On 07.06.2011 21:39, Mark Thomas wrote:
>> On 07/06/2011 20:24, Jess Holle wrote:
>>> web apps whose web.xml does not specify distributable should certainly
>>> not be treated as such -- that would be a spec violation and break lots
>>> of web apps.
>>
>> How many times do I have to write this? This is NOT what is being
>> proposed. All the proposed change does is fix a regression that prevents
>> *any* web application for declaring itself as distributable.
> 
> +1 to that change and to 7.0.15 is broken.

I've confirmed with testing that the proposed fix does exactly what is
intended and fixes the regression. I'll be committing it shortly.

Mark




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PROPOSAL] Enable session replication by default.

Posted by Rainer Jung <ra...@kippdata.de>.
On 07.06.2011 21:39, Mark Thomas wrote:
> On 07/06/2011 20:24, Jess Holle wrote:
>> web apps whose web.xml does not specify distributable should certainly
>> not be treated as such -- that would be a spec violation and break lots
>> of web apps.
> 
> How many times do I have to write this? This is NOT what is being
> proposed. All the proposed change does is fix a regression that prevents
> *any* web application for declaring itself as distributable.

+1 to that change and to 7.0.15 is broken.

Regards,

Rainer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PROPOSAL] Enable session replication by default.

Posted by Mark Thomas <ma...@apache.org>.
On 07/06/2011 20:24, Jess Holle wrote:
> web apps whose web.xml does not specify distributable should certainly
> not be treated as such -- that would be a spec violation and break lots
> of web apps.

How many times do I have to write this? This is NOT what is being
proposed. All the proposed change does is fix a regression that prevents
*any* web application for declaring itself as distributable.

Mark

> 
> On 6/7/2011 2:08 PM, Christopher Schultz wrote:
>> Mark,
>>
>> On 6/7/2011 2:36 PM, Mark Thomas wrote:
>>> On 07/06/2011 19:30, Christopher Schultz wrote:
>>>> Keiichi,
>>>>
>>>> On 6/7/2011 7:37 AM, Keiichi Fujino wrote:
>>>>> Index: conf/web.xml
>>>>> ===================================================================
>>>>> --- conf/web.xml    (revision 1127122)
>>>>> +++ conf/web.xml    (working copy)
>>>>> @@ -4176,4 +4176,6 @@
>>>>>           <welcome-file>index.jsp</welcome-file>
>>>>>       </welcome-file-list>
>>>>>
>>>>> +<!-- ====================  distributable ===================== -->
>>>>> +<distributable/>
>>>>>   </web-app>
>>>> -1
>>>>
>>>> This will cause web applications to fail when adding session attributes
>>>> that are not Serializable.
>>> Not it won't. This is *not* changing the default value of distributable.
>> That<distributable>  flag is in conf/web.xml, which I assumed set
>> defaults for all web applications. Is<distributable>  handled
>> differently than, say,<servlet-mapping>  or anything else we put into
>> conf/web.xml?
>>
>> I apologize for not having read the code at this point.
>>
>> -chris
>>
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PROPOSAL] Enable session replication by default.

Posted by Jess Holle <je...@ptc.com>.
web apps whose web.xml does not specify distributable should certainly 
not be treated as such -- that would be a spec violation and break lots 
of web apps.

On 6/7/2011 2:08 PM, Christopher Schultz wrote:
> Mark,
>
> On 6/7/2011 2:36 PM, Mark Thomas wrote:
>> On 07/06/2011 19:30, Christopher Schultz wrote:
>>> Keiichi,
>>>
>>> On 6/7/2011 7:37 AM, Keiichi Fujino wrote:
>>>> Index: conf/web.xml
>>>> ===================================================================
>>>> --- conf/web.xml	(revision 1127122)
>>>> +++ conf/web.xml	(working copy)
>>>> @@ -4176,4 +4176,6 @@
>>>>           <welcome-file>index.jsp</welcome-file>
>>>>       </welcome-file-list>
>>>>
>>>> +<!-- ====================  distributable ===================== -->
>>>> +<distributable/>
>>>>   </web-app>
>>> -1
>>>
>>> This will cause web applications to fail when adding session attributes
>>> that are not Serializable.
>> Not it won't. This is *not* changing the default value of distributable.
> That<distributable>  flag is in conf/web.xml, which I assumed set
> defaults for all web applications. Is<distributable>  handled
> differently than, say,<servlet-mapping>  or anything else we put into
> conf/web.xml?
>
> I apologize for not having read the code at this point.
>
> -chris
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PROPOSAL] Enable session replication by default.

Posted by Mark Thomas <ma...@apache.org>.
On 07/06/2011 20:08, Christopher Schultz wrote:
> Mark,
> 
> On 6/7/2011 2:36 PM, Mark Thomas wrote:
>> On 07/06/2011 19:30, Christopher Schultz wrote:
>>> Keiichi,
>>>
>>> On 6/7/2011 7:37 AM, Keiichi Fujino wrote:
>>>> Index: conf/web.xml
>>>> ===================================================================
>>>> --- conf/web.xml	(revision 1127122)
>>>> +++ conf/web.xml	(working copy)
>>>> @@ -4176,4 +4176,6 @@
>>>>          <welcome-file>index.jsp</welcome-file>
>>>>      </welcome-file-list>
>>>>
>>>> +  <!-- ====================  distributable ===================== -->
>>>> +    <distributable/>
>>>>  </web-app>
>>>
>>> -1
>>>
>>> This will cause web applications to fail when adding session attributes
>>> that are not Serializable.
>>
>> Not it won't. This is *not* changing the default value of distributable.
> 
> That <distributable> flag is in conf/web.xml, which I assumed set
> defaults for all web applications. Is <distributable> handled
> differently than, say, <servlet-mapping> or anything else we put into
> conf/web.xml?

Yes. The defaults are now treated as a fragment. When merging the main
web.xml with fragments, all the fragments and the main web.xml have to
have to be distributable for the result to be distributable.

The default distributable setting for main web.xml is false.
The default distributable setting for web-fragment.xml in JARs is false.

By default, distributable is therefore false.
Only if the main web.xml and all of the fragments are marked as
distributable will the application be distributable.

The defaults are marked as distributable (in code) so they do not
prevent an app from being distributable.

> I apologize for not having read the code at this point.

I forgive you. This time. :)

Mark

> 
> -chris
> 




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PROPOSAL] Enable session replication by default.

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,

On 6/7/2011 2:36 PM, Mark Thomas wrote:
> On 07/06/2011 19:30, Christopher Schultz wrote:
>> Keiichi,
>>
>> On 6/7/2011 7:37 AM, Keiichi Fujino wrote:
>>> Index: conf/web.xml
>>> ===================================================================
>>> --- conf/web.xml	(revision 1127122)
>>> +++ conf/web.xml	(working copy)
>>> @@ -4176,4 +4176,6 @@
>>>          <welcome-file>index.jsp</welcome-file>
>>>      </welcome-file-list>
>>>
>>> +  <!-- ====================  distributable ===================== -->
>>> +    <distributable/>
>>>  </web-app>
>>
>> -1
>>
>> This will cause web applications to fail when adding session attributes
>> that are not Serializable.
> 
> Not it won't. This is *not* changing the default value of distributable.

That <distributable> flag is in conf/web.xml, which I assumed set
defaults for all web applications. Is <distributable> handled
differently than, say, <servlet-mapping> or anything else we put into
conf/web.xml?

I apologize for not having read the code at this point.

-chris


Re: [PROPOSAL] Enable session replication by default.

Posted by Mark Thomas <ma...@apache.org>.
On 07/06/2011 19:30, Christopher Schultz wrote:
> Keiichi,
> 
> On 6/7/2011 7:37 AM, Keiichi Fujino wrote:
>> Index: conf/web.xml
>> ===================================================================
>> --- conf/web.xml	(revision 1127122)
>> +++ conf/web.xml	(working copy)
>> @@ -4176,4 +4176,6 @@
>>          <welcome-file>index.jsp</welcome-file>
>>      </welcome-file-list>
>>
>> +  <!-- ====================  distributable ===================== -->
>> +    <distributable/>
>>  </web-app>
> 
> -1
> 
> This will cause web applications to fail when adding session attributes
> that are not Serializable.

Not it won't. This is *not* changing the default value of distributable.

Mark



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PROPOSAL] Enable session replication by default.

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Keiichi,

On 6/7/2011 7:37 AM, Keiichi Fujino wrote:
> Index: conf/web.xml
> ===================================================================
> --- conf/web.xml	(revision 1127122)
> +++ conf/web.xml	(working copy)
> @@ -4176,4 +4176,6 @@
>          <welcome-file>index.jsp</welcome-file>
>      </welcome-file-list>
> 
> +  <!-- ====================  distributable ===================== -->
> +    <distributable/>
>  </web-app>

-1

This will cause web applications to fail when adding session attributes
that are not Serializable.

-chris


Re: [PROPOSAL] Enable session replication by default.

Posted by Mark Thomas <ma...@apache.org>.
On 07/06/2011 12:37, Keiichi Fujino wrote:
> Hi All.
> 
> Tomcat7.0.15 can not replicate session if web application defines
> <distributable/> in WEB-INF/web.xml.
> Tomcat7.0.14 can replicate session if web application defines
> <distributable/> in WEB-INF/web.xml.
> The behavior of this default was changed by rev1130618.
> The cause is conf/web.xml is not distributable.

That is enough to stop the 7.0.15 release in my view.

> To enable session replication by default,
> I thought two plans.
> 
> PlanA:
> Add WebXml#setDistributable(true) to ContextConfig#webConfig().
> 
> Index: java/org/apache/catalina/startup/ContextConfig.java
> ===================================================================
> --- java/org/apache/catalina/startup/ContextConfig.java	(revision 1132530)
> +++ java/org/apache/catalina/startup/ContextConfig.java	(working copy)
> @@ -1195,6 +1195,7 @@
>           */
>          WebXml webXmlDefaultFragment = createWebXml();
>          webXmlDefaultFragment.setOverridable(true);
> +        webXmlDefaultFragment.setDistributable(true);
> 
>          // Parse global web.xml if present
>          InputSource globalWebXml = getGlobalWebXmlSource();

<snip/>

> I intend to commit planA.
> Any objections or other ideas?

None here.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PROPOSAL] Enable session replication by default.

Posted by Mark Thomas <ma...@apache.org>.
On 07/06/2011 16:21, Konstantin Kolinko wrote:
> 2011/6/7 Keiichi Fujino <kf...@apache.org>:
>> Hi All.
>>
>> Tomcat7.0.15 can not replicate session if web application defines
>> <distributable/> in WEB-INF/web.xml.
>> Tomcat7.0.14 can replicate session if web application defines
>> <distributable/> in WEB-INF/web.xml.
>> The behavior of this default was changed by rev1130618.
>> The cause is conf/web.xml is not distributable.
>>
>> To enable session replication by default,
>> I thought two plans.
>>
>> PlanA:
>> PlanB:
> 
> -1
> 
> IIRC configuring the webapp as <distributable/> has observable
> consequence that each attribute assigned to a session will be checked
> whether it implements of the Serializable interface. It will mean that
> some webapps will stop working in the default configuration.
> 
> Thus we cannot assume that "distributable" flag is true by default.  I
> suspect that specification implies that "distributable" flag defaults
> to false. I have not checked the text though.

The proposed patch doesn't do that. It marks the defaultFragment as
distributable which means that the main web.xml file still determines
the final state of distributable.

Without marking the defaultFragment as distributable, a web-app can
never be distributable.

Mark



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PROPOSAL] Enable session replication by default.

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/6/7 Keiichi Fujino <kf...@apache.org>:
> Hi All.
>
> Tomcat7.0.15 can not replicate session if web application defines
> <distributable/> in WEB-INF/web.xml.
> Tomcat7.0.14 can replicate session if web application defines
> <distributable/> in WEB-INF/web.xml.
> The behavior of this default was changed by rev1130618.
> The cause is conf/web.xml is not distributable.
>
> To enable session replication by default,
> I thought two plans.
>
> PlanA:
> PlanB:

-1

IIRC configuring the webapp as <distributable/> has observable
consequence that each attribute assigned to a session will be checked
whether it implements of the Serializable interface. It will mean that
some webapps will stop working in the default configuration.

Thus we cannot assume that "distributable" flag is true by default.  I
suspect that specification implies that "distributable" flag defaults
to false. I have not checked the text though.


Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org