You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Gabriel Gruber <Ga...@workflow.at> on 2009/01/05 21:54:46 UTC

SSF Secrets or valid enhancements?

Hello Folks,

over the holidays I continued my last years playings with Apache Wicket 
and Cocoon. This time a little bit more successful. I have a working 
prototype now, where a Wicket Servlet is called by the Servlet Service 
Framework and the resulting xhtml is postprocessed by some XSLTs inside 
cocoon. Even Wicket Ajax is working fine!

However in order to get it running I had to make some changes in SSF, 
because I discovered these problems:

a) I found no way to foreward the request parameters from the original 
HTTP POST Request to the Wicket Servlet, alltough my pipelines looked 
correct. But after debugging I discovered that the created 
ServletServiceRequest object just didn't contain the request parameters of 
the original request.

b) SSF is not capeable of handling redirection of called HTTP servlets 
correctly. (HTTP Response 302)

c) some Methods of SSFs 'ServletServiceRespose' class are not implemented, 
but needed by the Wicket Servlet, namely: encodeRedirectUrl and 
encodeUrl..

In the end it turned out that b) is not a problem, if you change the 
RequestCycleStrategy inside Wicket (to use not redirection at all...).

So in order to fix these issues for me I had to make some changes to 
cocoon-servlet-service-impl and cocoon-servlet-service-components. While 
this patches worked for my usecase I would like to see those changes 
integrated into trunk obviously ;-)

I case of the ServletServiceRequest object I was a bit puzzled because in 
different posts on the mailing list I read that the "... req params & 
attrs  and session are passed/shared..." which was the thing I needed. 
however it didnot work for me. Looking at the code it seemed to me that 
only request parameters which where appended to the called URL (like 
myServlet?param1=a&param2=b)where actually passed, while does which were 
POSTED where NOT passed.

As the current behavior is somehow different to my new implementation, I 
suggest to implement a marker to switch between old and new behavior...

    public String getParameter(String name) {
 
        if (useParentRequest) 
                return (String) this.parentRequest.getParameter(name);
 
        return (String) this.parameters.getValue(name);
    }

How this marker could be implemented? F.i. via another special Postfix in 
the servlet-Name (like the one which is used to identify full qualified 
servlet-class-names)

If I am wrong with my assumptions, please tell me how I can reach the 
desired result without any modifications of this cocoon classes.

Attached are my patches...



WDYT?

By the way if it is interesting for the public I can post my wicket-cocoon 
integration sample for others!

cheers,

Gabriel

______________________
Mag. Gabriel Gruber
Senior Consultant
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Workflow EDV GmbH, Dannebergplatz 6/23, A-1030 Wien
----- Forwarded by Gabriel Gruber/Workflow on 05.01.2009 21:31 -----

"Reinhard Poetz (JIRA)" <ji...@apache.org> 
11.03.2008 09:11
Please respond to
dev@cocoon.apache.org


To
dev@cocoon.apache.org
cc

Subject
[jira] Closed: (COCOON-1831) Passing parameters to sub calls







     [ 
https://issues.apache.org/jira/browse/COCOON-1831?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel 
]

Reinhard Poetz closed COCOON-1831.
----------------------------------

    Resolution: Fixed

patch applied. req params & attrs  and session are passed/shared.

> Passing parameters to sub calls
> -------------------------------
>
>                 Key: COCOON-1831
>                 URL: https://issues.apache.org/jira/browse/COCOON-1831
>             Project: Cocoon
>          Issue Type: New Feature
>          Components: - Servlet service framework
>            Reporter: Reinhard Poetz
>            Assignee: Reinhard Poetz
>         Attachments: BlockCallHttpServletRequest.patch, 
cocoon-servlet-service-impl.patch, cocoon-servlet-service-impl.patch
>
>
> When a servlet service request is created, parameters from the parent 
request are ignored. This means that the sub request is performed as a 
fresh and clean new call. This would avoid any possible side-effects, but 
is very inconvenient in practice because you don't even know the request 
header parameters from the original (external) request. Additionally you 
can only pass information which is part of the returned stream, which is 
e.g. a  blocker to use the servlet protocol together with the control flow 
implementations. Those make use of special request parameters to transport 
the model ("bizdata") to the view layer. 

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


Re: SSF Secrets or valid enhancements?

Posted by Grzegorz Kossakowski <gr...@tuffmail.com>.
Gabriel Gruber pisze:
> 
> Hello Folks,

Hello Gabriel,

> over the holidays I continued my last years playings with Apache Wicket
> and Cocoon. This time a little bit more successful. I have a working
> prototype now, where a Wicket Servlet is called by the Servlet Service
> Framework and the resulting xhtml is postprocessed by some XSLTs inside
> cocoon. Even Wicket Ajax is working fine!

Nice. We've been discussing such a setup a long time ago (two years ago?) but nobody really cared to
implement this. I'm glad to see that SSF can work as we designed it. :-) (kudos to Daniel Fagerstrom)

> However in order to get it running I had to make some changes in SSF,
> because I discovered these problems:
> 
> a) I found no way to foreward the request parameters from the original
> HTTP POST Request to the Wicket Servlet, alltough my pipelines looked
> correct. But after debugging I discovered that the created
> ServletServiceRequest object just didn't contain the request parameters
> of the original request.
> 
> b) SSF is not capeable of handling redirection of called HTTP servlets
> correctly. (HTTP Response 302)

What do you mean exactly? SSF will pass this kind of response back to browser. Would you expect it
do something else?

> c) some Methods of SSFs 'ServletServiceRespose' class are not
> implemented, but needed by the Wicket Servlet, namely: encodeRedirectUrl
> and encodeUrl..

Yep, SSF contains quite a lot of TODO markers. Most of these methods are very straightforward to
implement. Patches are welcome.

> In the end it turned out that b) is not a problem, if you change the
> RequestCycleStrategy inside Wicket (to use not redirection at all...).
> 
> So in order to fix these issues for me I had to make some changes to
> cocoon-servlet-service-impl and cocoon-servlet-service-components. While
> this patches worked for my usecase I would like to see those changes
> integrated into trunk obviously ;-)
> 
> I case of the ServletServiceRequest object I was a bit puzzled because
> in different posts on the mailing list I read that the "... req params &
> attrs  and session are passed/shared..." which was the thing I needed.
> however it didnot work for me. Looking at the code it seemed to me that
> only request parameters which where appended to the called URL (like
> myServlet?param1=a&param2=b)where actually passed, while does which were
> POSTED where NOT passed.

Strictly speaking, parameters sent using POST method are not parameters in HTTP spec sense as far as
I remember. Anyway, from Servlets are handling these both methods in an unified way.

The main reason why this is not happening is that it's not that easy to implement, see
http://thread.gmane.org/gmane.text.xml.cocoon.devel/73088/focus=73137 and answers to this post which
tries to clarify the problem.

> As the current behavior is somehow different to my new implementation, I
> suggest to implement a marker to switch between old and new behavior...
> 
>     *public* String getParameter(String name) {
>            
>             *if* (useParentRequest)                          
>                     *return* (String)
> *this*.parentRequest.getParameter(name);
>            
>         *return* (String) *this*.parameters.getValue(name);
>     }
> 
> How this marker could be implemented? F.i. via another special Postfix
> in the servlet-Name (like the one which is used to identify full
> qualified servlet-class-names)

-1
Current implementation is not final one and if we break back-compatibility slightly in order to make
implementation complete I'm all for it.

> If I am wrong with my assumptions, please tell me how I can reach the
> desired result without any modifications of this cocoon classes.

As you see it has to be modified and it's been already discussed but the solution is not that simple
and requires careful consideration of all possible cases.

> Attached are my patches...

Comments inline.

> ### Eclipse Workspace Patch 1.0
> #P cocoon-servlet-service-components
> Index: src/main/java/org/apache/cocoon/servletservice/components/ServletSource.java
> ===================================================================
> --- src/main/java/org/apache/cocoon/servletservice/components/ServletSource.java	(revision 731081)
> +++ src/main/java/org/apache/cocoon/servletservice/components/ServletSource.java	(working copy)
> @@ -84,7 +84,8 @@
>                         + "The servlet returned " + servletConnection.getResponseCode() + " response code.");
>              
>              // FIXME: This is not the most elegant solution
> -            if (servletConnection.getResponseCode() != HttpServletResponse.SC_OK) {
> +            int rc = servletConnection.getResponseCode(); 
> +            if (rc != HttpServletResponse.SC_OK) {
>                  //most probably, servlet returned 304 (not modified) and we need to perform second request to get data
>  
>                  //
> @@ -94,7 +95,15 @@
>                  //       and, as a result, GET request instead of POST.
>                  //
>  
> -                servletConnection = createServletConnection(location);
> +            	if (rc == HttpServletResponse.SC_FOUND) {

Using SC_MOVED_TEMPORARILY is preferred as it's more obvious and stays close to terminology used in
HTTP spec.
What about SC_MOVED_PERMANENTLY?

These two redirects should be handled in unified way as long as you don't deal with caching as
defined in HTTP spec.

> +            		// if request sent a redirect status code
> +            		// we shall connect to the new location
> +            		String redirectUrl = servletConnection.getRedirectURI().toString();
> +            		servletConnection = createServletConnection(redirectUrl);

Here you assume that getRedirectURI() returns *internal* URI right (with servlet: protocol)? Without
testing this assumption is wrong.

> +            	} else {
> +            		servletConnection = createServletConnection(location);
> +            	}
> +            	
>                  servletConnection.connect();
>              }


> ### Eclipse Workspace Patch 1.0
> #P cocoon-servlet-service-impl
> Index: src/main/java/org/apache/cocoon/servletservice/util/ServletServiceResponse.java
> ===================================================================
> --- src/main/java/org/apache/cocoon/servletservice/util/ServletServiceResponse.java	(revision 731081)
> +++ src/main/java/org/apache/cocoon/servletservice/util/ServletServiceResponse.java	(working copy)
> @@ -20,6 +20,7 @@
>  import java.io.OutputStream;
>  import java.io.OutputStreamWriter;
>  import java.io.PrintWriter;
> +import java.net.URI;
>  import java.text.ParseException;
>  import java.text.SimpleDateFormat;
>  import java.util.Date;
> @@ -31,6 +32,7 @@
>  import javax.servlet.http.Cookie;
>  import javax.servlet.http.HttpServletResponse;
>  
> +
>  /**
>   * Creates a {@link HttpServletResponse} object that is usable for internal block calls.
>   *
> @@ -45,6 +47,7 @@
>      private boolean committed;
>      private Locale locale;
>      private int statusCode;
> +    private URI url;

This is non-obvious variable as response usually does not care about to which url it is the
response. Some comment would be useful why it's needed.

>      private Map headers;
>  
> @@ -53,6 +56,11 @@
>       */
>      final SimpleDateFormat dateFormat = new SimpleDateFormat("EEE', 'dd' 'MMM' 'yyyy' 'HH:mm:ss' 'Z", Locale.US);
>  
> +    public ServletServiceResponse(URI url) {
> +    	this.url = url;
> +        headers = new HashMap();
> +        statusCode = HttpServletResponse.SC_OK;
> +    }
>  
>      public ServletServiceResponse() {
>          headers = new HashMap();
> @@ -83,23 +91,19 @@
>      }
>  
>      public String encodeRedirectUrl(String url) {
> -        // TODO Auto-generated method stub
> -        return null;
> +        return url;
>      }
>  
>      public String encodeRedirectURL(String url) {
> -        // TODO Auto-generated method stub
> -        return null;
> +        return url;
>      }
>  
>      public String encodeUrl(String url) {
> -        // TODO Auto-generated method stub
> -        return null;
> +        return url;
>      }
>  
>      public String encodeURL(String url) {
> -        // TODO Auto-generated method stub
> -        return null;
> +        return url;
>      }

Quick check of Javadocs[1] reveals:
Encodes the specified URL by including the session ID in it, or, if encoding is not needed, returns
the URL unchanged.

I can't see an argument for assumption that encoding is not needed here. Could you elaborate?

>      public void flushBuffer() throws IOException {
> @@ -191,8 +195,39 @@
>      }
>  
>      public void sendRedirect(String location) throws IOException {
> -        // TODO Auto-generated method stub
> +    	
> +        if (!location.startsWith("servlet:"))
> +        {
> +            StringBuffer buf = getRootURL(url);
> +//            if (location.startsWith("/"))
> +//                buf.append(URIUtil.canonicalPath(location));
> +//            else
> +//            {
> +//                String path=url.getPath();
> +//                String parent=(path.endsWith("/"))?path:URIUtil.parentPath(path);
> +//                location=URIUtil.canonicalPath(URIUtil.addPaths(parent,location));
> +//                if (!location.startsWith("/"))
> +//                    buf.append('/');
> +//                buf.append(location);
> +//            }
> +            buf.append(location);
> +            location=buf.toString();
> +        }
> +    	
> +    	// build redirect location
> +    	setHeader("Location",location);
> +        setStatus(HttpServletResponse.SC_MOVED_TEMPORARILY);
>      }
> +    
> +    public StringBuffer getRootURL(URI uri)
> +    {
> +        StringBuffer url = new StringBuffer();
> +        String scheme = uri.getScheme();
> +        url.append(scheme);
> +        url.append(":");
> +    	url.append(uri.getSchemeSpecificPart());
> +        return url;
> +    }

Cannot parse this fragment. Why ServletServiceRepsonse has to care about protocol of location
argument in sendRedirect()?

>      public void setBufferSize(int size) {
>          // TODO Implement buffering, for the moment ignore.
> Index: src/main/java/org/apache/cocoon/servletservice/AbsoluteServletConnection.java
> ===================================================================
> --- src/main/java/org/apache/cocoon/servletservice/AbsoluteServletConnection.java	(revision 731081)
> +++ src/main/java/org/apache/cocoon/servletservice/AbsoluteServletConnection.java	(working copy)
> @@ -77,7 +77,7 @@
>              throw iae;
>          }
>          this.request = new ServletServiceRequest(reqUri, CallFrameHelper.getRequest());
> -        this.response = new ServletServiceResponse();
> +        this.response = new ServletServiceResponse(this.uri);
>      }
>  
>      /**
> Index: src/main/java/org/apache/cocoon/servletservice/ServletConnection.java
> ===================================================================
> --- src/main/java/org/apache/cocoon/servletservice/ServletConnection.java	(revision 731081)
> +++ src/main/java/org/apache/cocoon/servletservice/ServletConnection.java	(working copy)
> @@ -113,6 +113,12 @@
>       * @return a URI representing this servlet connection.
>       */
>      URI getURI();
> +    
> +    /**
> +    * Get a URI representing the redirect URI incase of responsecode of 302
> +    * 
> +    * @return a URI representing the redirect servlet connection
> +    */
> +   URI getRedirectURI();
>  
> -
>  }
> Index: src/main/java/org/apache/cocoon/servletservice/util/ServletServiceRequest.java
> ===================================================================
> --- src/main/java/org/apache/cocoon/servletservice/util/ServletServiceRequest.java	(revision 731081)
> +++ src/main/java/org/apache/cocoon/servletservice/util/ServletServiceRequest.java	(working copy)
> @@ -123,6 +123,8 @@
>      private Parameters parameters;
>  
>      private ServletServiceContext context;
> +    
> +    private boolean useParentRequest = true;
>  
>      /**
>       * @param uri
> @@ -255,18 +257,31 @@
>      //
>  
>      public String getParameter(String name) {
> +    	
> +    	if (useParentRequest)  			
> +    		return (String) this.parentRequest.getParameter(name);
> +    	
>          return (String) this.parameters.getValue(name);
>      }
>  
>      public String[] getParameterValues(String name) {
> +    	if (useParentRequest)
> +    		return this.parentRequest.getParameterValues(name);
> +    	
>          return this.parameters.getValues(name);
>      }
>  
>      public Enumeration getParameterNames() {
> +    	if (useParentRequest)
> +    		return this.parentRequest.getParameterNames();
> +    	
>          return this.parameters.getNames();
>      }
>  
>      public Map getParameterMap() {
> +    	if (useParentRequest)
> +    		return this.parentRequest.getParameterMap();
> +    	
>          return this.parameters.getValues();
>      }

I've already given my opinion on using special marker. Would like to hear what others think about it.

> Index: src/main/java/org/apache/cocoon/servletservice/AbstractServletConnection.java
> ===================================================================
> --- src/main/java/org/apache/cocoon/servletservice/AbstractServletConnection.java	(revision 731081)
> +++ src/main/java/org/apache/cocoon/servletservice/AbstractServletConnection.java	(working copy)
> @@ -22,6 +22,7 @@
>  import java.io.InputStream;
>  import java.io.OutputStream;
>  import java.net.URI;
> +import java.net.URISyntaxException;
>  
>  import javax.servlet.ServletContext;
>  import javax.servlet.ServletException;
> @@ -194,5 +195,15 @@
>          }
>  
>      }
> +    
> +    public URI getRedirectURI() {
> +    	String redirectLocation = this.response.getHeader("Location");
> +    	try {
> +			return new URI(redirectLocation);
> +		} catch (URISyntaxException e) {
> +			this.logger.error("problems creating correct URI...", e);
> +			return null;
> +		}
> +    }
>  
>  }
> Index: src/main/java/org/apache/cocoon/servletservice/RelativeServletConnection.java
> ===================================================================
> --- src/main/java/org/apache/cocoon/servletservice/RelativeServletConnection.java	(revision 731081)
> +++ src/main/java/org/apache/cocoon/servletservice/RelativeServletConnection.java	(working copy)
> @@ -79,7 +79,7 @@
>  
>          // prepare request and response objects
>          this.request = new ServletServiceRequest(reqUri, CallFrameHelper.getRequest());
> -        this.response = new ServletServiceResponse();
> +        this.response = new ServletServiceResponse(reqUri);
>  
>          if(this.logger.isDebugEnabled()) {
>              this.logger.debug("Resolving relative servlet URI " + this.uri.toASCIIString());

There are a few more minor things to comment on but for this iteration let's focus on most important
stuff.

> WDYT?
> 
> By the way if it is interesting for the public I can post my
> wicket-cocoon integration sample for others!

Some folks asked for it in the past and we considered this ourselves so I think this is interesting
to others.

Thanks for your offer and for work on implementing missing pieces of SSF.


-- 
Best regards,
Grzegorz Kossakowski