You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Felix Meschberger (JIRA)" <ji...@apache.org> on 2010/01/06 11:37:54 UTC

[jira] Commented: (SLING-1270) Replace Session.logout from SlingMainServlet

    [ https://issues.apache.org/jira/browse/SLING-1270?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12797052#action_12797052 ] 

Felix Meschberger commented on SLING-1270:
------------------------------------------

Committed temporary session logout in the SlingMainServlet in Rev. 896372.

> Replace Session.logout from SlingMainServlet
> --------------------------------------------
>
>                 Key: SLING-1270
>                 URL: https://issues.apache.org/jira/browse/SLING-1270
>             Project: Sling
>          Issue Type: Task
>          Components: Engine
>    Affects Versions: Engine 2.0.6
>            Reporter: Felix Meschberger
>            Assignee: Felix Meschberger
>             Fix For: Engine 2.1.0
>
>
> The new Commons Auth bundle from SLING-966 registers a ServletRequestListener to be informed when the request has terminated and the session may be logged out. Currently, the Http Service implementation does not support such listeners and the session may not be logged out at all.
> As a workaround the Commons Auth bundle implements a Java VM finalize() method to try to ensure logging the session out.
> As a further workaround the SlingMainServlet should - in a finally clause - logout the session of the request's resource resolver.
> The SlingMainServlet configuration should be removed as soon as we can reasonably be sure of ServletRequestListener support.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Re: Sling Request Lifecycle events (was: [jira] Commented: (SLING-1270) Replace Session.logout from SlingMainServlet)

Posted by Ian Boston <ie...@tfd.co.uk>.
On 6 Jan 2010, at 13:01, Felix Meschberger wrote:

> Hi Ian,
> 
> On 06.01.2010 11:57, Ian Boston wrote:
>> I agree that we should use the ServletRequestListener support, but I continue to wonder if there isnt a need for a request lifecycle to be exposed by the sling main servlet which would avoid having to a) create a stack of ServletFilters b) bind the SlingMainServlet to other apis' c) bind to the ServletRequestListener which is as you note not always supported ?
> 
> We might want to add request lifecycle events to the SlingMainServlet as
> far as the service method is concerned. This would work perfectly fine
> for requests handled through the SlingMainServlet (which is the majority
> of the requests handled by a Sling instance).
> 
> Specifically the WebDAV requests to the /dav subtree would not benefit
> from this (unless we add the same lifecycle support there ...)
> 
> Still: this would not help in the authentication case because
> authentication is handled outside of the SlingMainServlet before the
> service method is even called. In fact if authentication fails, the
> SlingMainServlet.service method is not called at all.


good point.

> 
>> 
>> At present we have ServletFilters for a XA Transaction manager and Request Scope Caching, the stack is good in some senses since we can perform try finally but we rely heavily on the programmer knowing exactly what is safe to do.
> 
> How about adding a SlingRequestListener interface, which mimicks the
> ServletRequestListener interface:
> 
>   public interface SlingRequestListener {
>        void requestDestroyed(SlingRequestEvent sre);
>        void requestInitialized(SlingRequestEvent sre);
>   }
> 
>   public class SlingRequestEvent {
>        SlingHttpServletRequest getSlingRequest();
>   }
> 
> The methods would be called at the same time, the RequestLogger methods
> are called (lines 252 and 363 in trunk SlingMainServlet).
> 
> WDYT ?


yes that makes perfect sense, the calls obviously wrapped in try catch to prevent anything doing bad things on destroy, ie the event is only for notification purposes and not for flow control etc

Ian

> 
> Regards
> Felix
> 
>> 
>> Ian
>> 
>> On 6 Jan 2010, at 10:37, Felix Meschberger (JIRA) wrote:
>> 
>>> 
>>>   [ https://issues.apache.org/jira/browse/SLING-1270?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12797052#action_12797052 ] 
>>> 
>>> Felix Meschberger commented on SLING-1270:
>>> ------------------------------------------
>>> 
>>> Committed temporary session logout in the SlingMainServlet in Rev. 896372.
>>> 
>>>> Replace Session.logout from SlingMainServlet
>>>> --------------------------------------------
>>>> 
>>>>               Key: SLING-1270
>>>>               URL: https://issues.apache.org/jira/browse/SLING-1270
>>>>           Project: Sling
>>>>        Issue Type: Task
>>>>        Components: Engine
>>>>  Affects Versions: Engine 2.0.6
>>>>          Reporter: Felix Meschberger
>>>>          Assignee: Felix Meschberger
>>>>           Fix For: Engine 2.1.0
>>>> 
>>>> 
>>>> The new Commons Auth bundle from SLING-966 registers a ServletRequestListener to be informed when the request has terminated and the session may be logged out. Currently, the Http Service implementation does not support such listeners and the session may not be logged out at all.
>>>> As a workaround the Commons Auth bundle implements a Java VM finalize() method to try to ensure logging the session out.
>>>> As a further workaround the SlingMainServlet should - in a finally clause - logout the session of the request's resource resolver.
>>>> The SlingMainServlet configuration should be removed as soon as we can reasonably be sure of ServletRequestListener support.
>>> 
>>> -- 
>>> This message is automatically generated by JIRA.
>>> -
>>> You can reply to this email to add a comment to the issue online.
>>> 
>> 
>> 


Sling Request Lifecycle events (was: [jira] Commented: (SLING-1270) Replace Session.logout from SlingMainServlet)

Posted by Felix Meschberger <fm...@gmail.com>.
Hi Ian,

On 06.01.2010 11:57, Ian Boston wrote:
> I agree that we should use the ServletRequestListener support, but I continue to wonder if there isnt a need for a request lifecycle to be exposed by the sling main servlet which would avoid having to a) create a stack of ServletFilters b) bind the SlingMainServlet to other apis' c) bind to the ServletRequestListener which is as you note not always supported ?

We might want to add request lifecycle events to the SlingMainServlet as
far as the service method is concerned. This would work perfectly fine
for requests handled through the SlingMainServlet (which is the majority
of the requests handled by a Sling instance).

Specifically the WebDAV requests to the /dav subtree would not benefit
from this (unless we add the same lifecycle support there ...)

Still: this would not help in the authentication case because
authentication is handled outside of the SlingMainServlet before the
service method is even called. In fact if authentication fails, the
SlingMainServlet.service method is not called at all.

> 
> At present we have ServletFilters for a XA Transaction manager and Request Scope Caching, the stack is good in some senses since we can perform try finally but we rely heavily on the programmer knowing exactly what is safe to do.

How about adding a SlingRequestListener interface, which mimicks the
ServletRequestListener interface:

   public interface SlingRequestListener {
        void requestDestroyed(SlingRequestEvent sre);
        void requestInitialized(SlingRequestEvent sre);
   }

   public class SlingRequestEvent {
        SlingHttpServletRequest getSlingRequest();
   }

The methods would be called at the same time, the RequestLogger methods
are called (lines 252 and 363 in trunk SlingMainServlet).

WDYT ?

Regards
Felix

> 
> Ian
> 
> On 6 Jan 2010, at 10:37, Felix Meschberger (JIRA) wrote:
> 
>>
>>    [ https://issues.apache.org/jira/browse/SLING-1270?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12797052#action_12797052 ] 
>>
>> Felix Meschberger commented on SLING-1270:
>> ------------------------------------------
>>
>> Committed temporary session logout in the SlingMainServlet in Rev. 896372.
>>
>>> Replace Session.logout from SlingMainServlet
>>> --------------------------------------------
>>>
>>>                Key: SLING-1270
>>>                URL: https://issues.apache.org/jira/browse/SLING-1270
>>>            Project: Sling
>>>         Issue Type: Task
>>>         Components: Engine
>>>   Affects Versions: Engine 2.0.6
>>>           Reporter: Felix Meschberger
>>>           Assignee: Felix Meschberger
>>>            Fix For: Engine 2.1.0
>>>
>>>
>>> The new Commons Auth bundle from SLING-966 registers a ServletRequestListener to be informed when the request has terminated and the session may be logged out. Currently, the Http Service implementation does not support such listeners and the session may not be logged out at all.
>>> As a workaround the Commons Auth bundle implements a Java VM finalize() method to try to ensure logging the session out.
>>> As a further workaround the SlingMainServlet should - in a finally clause - logout the session of the request's resource resolver.
>>> The SlingMainServlet configuration should be removed as soon as we can reasonably be sure of ServletRequestListener support.
>>
>> -- 
>> This message is automatically generated by JIRA.
>> -
>> You can reply to this email to add a comment to the issue online.
>>
> 
> 

Re: [jira] Commented: (SLING-1270) Replace Session.logout from SlingMainServlet

Posted by Ian Boston <ie...@tfd.co.uk>.
Felix, 
I agree that we should use the ServletRequestListener support, but I continue to wonder if there isnt a need for a request lifecycle to be exposed by the sling main servlet which would avoid having to a) create a stack of ServletFilters b) bind the SlingMainServlet to other apis' c) bind to the ServletRequestListener which is as you note not always supported ?

At present we have ServletFilters for a XA Transaction manager and Request Scope Caching, the stack is good in some senses since we can perform try finally but we rely heavily on the programmer knowing exactly what is safe to do.

Ian

On 6 Jan 2010, at 10:37, Felix Meschberger (JIRA) wrote:

> 
>    [ https://issues.apache.org/jira/browse/SLING-1270?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12797052#action_12797052 ] 
> 
> Felix Meschberger commented on SLING-1270:
> ------------------------------------------
> 
> Committed temporary session logout in the SlingMainServlet in Rev. 896372.
> 
>> Replace Session.logout from SlingMainServlet
>> --------------------------------------------
>> 
>>                Key: SLING-1270
>>                URL: https://issues.apache.org/jira/browse/SLING-1270
>>            Project: Sling
>>         Issue Type: Task
>>         Components: Engine
>>   Affects Versions: Engine 2.0.6
>>           Reporter: Felix Meschberger
>>           Assignee: Felix Meschberger
>>            Fix For: Engine 2.1.0
>> 
>> 
>> The new Commons Auth bundle from SLING-966 registers a ServletRequestListener to be informed when the request has terminated and the session may be logged out. Currently, the Http Service implementation does not support such listeners and the session may not be logged out at all.
>> As a workaround the Commons Auth bundle implements a Java VM finalize() method to try to ensure logging the session out.
>> As a further workaround the SlingMainServlet should - in a finally clause - logout the session of the request's resource resolver.
>> The SlingMainServlet configuration should be removed as soon as we can reasonably be sure of ServletRequestListener support.
> 
> -- 
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>