You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Johan Stuyts <j....@hippo.nl> on 2004/12/02 16:14:05 UTC

RE: Possible security problem with flowscript

Hi,

Sorry about reacting to this thread after one month of inactivity, but we recently switched to a Cocoon version which includes this fix. I have tried refactoring our application, but it was more work than I expected.

Having an option to turn this behaviour on or off would really make things easier for me. Our application can run as it is and from the warnings in the log I can determine what should be changed. Using these warnings I can gradually make our application compliant with sitemap-bound continuations.

I propose to change the current code in ContinuationsManagerImpl to this code. As you can see the warnings will always be added to the log as an incentive to make changes to code which still uses shared continuations:
    public WebContinuation lookupWebContinuation(String id, String interpreterId) {
        WebContinuation kont = (WebContinuation) idToWebCont.get(id);
        if ( kont != null ) {
            boolean interpreterMatches = kont.interpreterMatches(interpreterId);
            if (!interpreterMatches && getLogger().isWarnEnabled()) {
                getLogger().warn("WK: Continuation (" + kont.getId() 
                                 + ") lookup for wrong interpreter. Bound to: " 
                                 + kont.getInterpreterId() + ", looked up for: " 
                                 + interpreterId);
            }
>>>>        return interpreterMatches || allowBackwardCompatibleContinuationSharing ? kont : null;
        }
        return null;
    }

'allowBackwardCompatibleContinuationSharing' will be a configuration option which defaults to 'false'.

If nobody objects I will make the changes, create patches and add a new bug for this.

Johan Stuyts
Hippo

> -----Original Message-----
> From: Carsten Ziegeler [mailto:cziegeler@apache.org]
> Posted At: donderdag 21 oktober 2004 9:49
> Posted To: Cocoon Dev List
> Conversation: Possible security problem with flowscript
> Subject: RE: Possible security problem with flowscript
> 
> 
>  
> Leszek Gawron wrote:
> > >>Is there any sense to bind continuations to the sitemap? Vadim?
> > >>
> > > 
> > > Yes, I really think so. IMHO it is simply wrong to continue 
> > a script 
> > > in a sitemap where it hasn't been declared - and as soon as 
> > the flow 
> > > script tries to address relative resources it won't work anyway.
> >
> > Just one more question: should this be an option to maintain 
> > compatibility?
> > 
> I don't think so, imho it's a bug - in most cases the script breaks
> as soon as a pipeline is invoked - or other relative resources are
> fetched.
> 
> Carsten
> 
> 

Re: Possible security problem with flowscript

Posted by Leszek Gawron <lg...@mobilebox.pl>.
Johan Stuyts wrote:
> Hi,
> 
> Sorry about reacting to this thread after one month of inactivity, but we recently switched to a Cocoon version which includes this fix. I have tried refactoring our application, but it was more work than I expected.
> 
> Having an option to turn this behaviour on or off would really make things easier for me. Our application can run as it is and from the warnings in the log I can determine what should be changed. Using these warnings I can gradually make our application compliant with sitemap-bound continuations.
> 
> I propose to change the current code in ContinuationsManagerImpl to this code. As you can see the warnings will always be added to the log as an incentive to make changes to code which still uses shared continuations:
>     public WebContinuation lookupWebContinuation(String id, String interpreterId) {
>         WebContinuation kont = (WebContinuation) idToWebCont.get(id);
>         if ( kont != null ) {
>             boolean interpreterMatches = kont.interpreterMatches(interpreterId);
>             if (!interpreterMatches && getLogger().isWarnEnabled()) {
>                 getLogger().warn("WK: Continuation (" + kont.getId() 
>                                  + ") lookup for wrong interpreter. Bound to: " 
>                                  + kont.getInterpreterId() + ", looked up for: " 
>                                  + interpreterId);
>             }
> 
>>>>>       return interpreterMatches || allowBackwardCompatibleContinuationSharing ? kont : null;
> 
>         }
>         return null;
>     }
> 
> 'allowBackwardCompatibleContinuationSharing' will be a configuration option which defaults to 'false'.
> 
> If nobody objects I will make the changes, create patches and add a new bug for this.

Fine for me. What I do not like is the 
allowBackwardCompatibleContinuationSharing as it indicates that backward 
imcompatible change was made while this was a bugfix really.

I will patch the code as soon as you open the issue.

-- 
Leszek Gawron                                      lgawron@mobilebox.pl
Project Manager                                    MobileBox sp. z o.o.
+48 (61) 855 06 67                              http://www.mobilebox.pl
mobile: +48 (501) 720 812                       fax: +48 (61) 853 29 65

Re: Possible security problem with flowscript

Posted by Leszek Gawron <lg...@mobilebox.pl>.
Johan Stuyts wrote:
> Hi,
> 
> Sorry about reacting to this thread after one month of inactivity, but we recently switched to a Cocoon version which includes this fix. I have tried refactoring our application, but it was more work than I expected.
> 
> Having an option to turn this behaviour on or off would really make things easier for me. Our application can run as it is and from the warnings in the log I can determine what should be changed. Using these warnings I can gradually make our application compliant with sitemap-bound continuations.
> 
> I propose to change the current code in ContinuationsManagerImpl to this code. As you can see the warnings will always be added to the log as an incentive to make changes to code which still uses shared continuations:
>     public WebContinuation lookupWebContinuation(String id, String interpreterId) {
>         WebContinuation kont = (WebContinuation) idToWebCont.get(id);
>         if ( kont != null ) {
>             boolean interpreterMatches = kont.interpreterMatches(interpreterId);
>             if (!interpreterMatches && getLogger().isWarnEnabled()) {
>                 getLogger().warn("WK: Continuation (" + kont.getId() 
>                                  + ") lookup for wrong interpreter. Bound to: " 
>                                  + kont.getInterpreterId() + ", looked up for: " 
>                                  + interpreterId);
>             }
> 
>>>>>       return interpreterMatches || allowBackwardCompatibleContinuationSharing ? kont : null;
> 
>         }
>         return null;
>     }
> 
> 'allowBackwardCompatibleContinuationSharing' will be a configuration option which defaults to 'false'.
> 
> If nobody objects I will make the changes, create patches and add a new bug for this.
> 
> Johan Stuyts

Johan,
Please describe your usecase when you were resuming continuations in 
different sitemap. What did you try to achieve by this?

-- 
Leszek Gawron                                      lgawron@mobilebox.pl
Project Manager                                    MobileBox sp. z o.o.
+48 (61) 855 06 67                              http://www.mobilebox.pl
mobile: +48 (501) 720 812                       fax: +48 (61) 853 29 65