You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Stefano Mazzocchi <st...@apache.org> on 2003/03/22 10:38:02 UTC

[RT] pipeline aspects

Geoff Howard wrote:

> Obviously, if my 
> application doesn't use any uploads I should disable them in web.xml.  
> But right now, it's all or nothing: I either allow all users to upload 
> _on any page_ (if they create a form that posts to any url in cocoon's 
> space), or I totally disallow uploads.  I've been thinking through 
> enabling configs for resource-based, or even authentication-based 
> restrictions for uploads.  What would others think?

I've been thinking about this for a while and I thought about waiting 
for 2.1 before sending these RT, but I just wanted to introduce them 
briefly before a quick-hack solution is implemented.

In my continuous quest to see if our pipeline architecture is enough for 
even the most complex web needs, I found out that it is not always the case.

After we release 2.1, I will introduce a discussion about "pipeline 
aspects" in order to suggest a more elegant approach to:

  - handling webdav
  - handling uploading
  - handling views
  - handling proxying
  - handling chunked encoding

In short, all those behaviors that require deep contact with the 
internals of the HTTP stream both in reading and writing and, 
potentialy, from all the pipeline components.

The way we designed the pipelines was mediated out of the servlet API 
and the servlet API was designed exactly to prevent you from having to 
know (or manage) the HTTP internals.

Since the approach was a very useful one (in short, clean IoC applied to 
the HTTP paradigm), we designed the environment in that direction.

But we have one major difference between the servlet API: the API is 
designed so that one or more web resource is associated to one (or, 
using request dispaching, a procedural chain of) programmatic request 
handlers (the servlets or the jsp pages).

Our model is entirely different and it's pretty rare (see readers) that 
we have only one request-handling component that touches the request.

Cocoon aims at complete and clean SoC and started for publishing needs 
(stateless, GET-driven). We achieved that with the sitemap. Then we 
moved into webapps (stateful, GET/POST roundtrip driven). I think we 
solved all possible issues with the flow.

The question I posed to myself was: now that we have both pipelines, 
sitemaps and flows, are we able to handle the complexity of WebDAV in a 
clean way?

The answer is, unfortunately, "no".

Don't get me wrong: you can write a cocoon-enabled webdav-app 
(webdav-apps are webapps that use not only GET/POST but all the webdav 
extension to the HTTP protocol!) even with 2.1 but it won't be so clean.

why?

well, unlike GET/POST actions which inherently assume a 
resource-oriented granularity, there are other actions (and I consider 
the cocoon view a special case of HTTP content negotiation) that require 
'cross cutting' behavior across different resources.

I think the sitemap semantics is inherently great at describing concerns 
that can be well separated (using matching, for example) but very poor 
at describing crosscutting concerns.

Examples of crosscutting concerns are:

  1) webdav enabled
  2) restricted/protected
  3) proxiable
  4) viewable
  5) upload handling
  6) pipeline-wide global parameters

There is no single mechanism to handle all those.

Views have a special semantics. Upload handling is made *explicitly* 
cross-cut across the entire domain space (and this is, rightly, a 
potential vulnerable point, expecially if associated with resource 
restriction).

The best example of crosscutting concern is Restriction/protection: this 
is most elegantly done (currently) with wrapping a map:mount.

I wonder is this is, architecturally, the best approach.

In theory, it should be possible to define 'behavior modifiers' that 
wrap pipelines and transparently provide different behaviors to them.

I see two possible approaches:

  - external modification
  - internal modification

external modification is done with wrapping a mount, like we do today 
for restricting access to an entire sub-sitemap.

internal modification is done for views, but this required a special 
semantics added to the sitemap and, in retrospect, it seems to me that 
it was a mistake.

We can't use Actions or flow to modify pipeline behaviors unless we give 
full access to the underlying object model, opening the gates for abuses.

We can't add the possibility for people to add their own sitemap 
semantics, or we have the same abuse problem.

I see two possible solutions, depending on where we want to go:

  1) create a 'wrapping' component

or

  2) add the ability to add pluggable <map:pipeline> metadata without 
exposing hooks for internal pipeline abuses.

Note that these are very random thoughts and I don't have a coherent 
view of what we need and how we solve those issue. at the same time, I 
think it's worth thinking at those pipeline-wide problems with little 
aspect orientation in mind.

Stefano.


Temp Upload start (See also [RT] pipeline aspects)

Posted by Geoff Howard <co...@leverageweb.com>.
> -----Original Message-----
> From: Stefano Mazzocchi [mailto:stefano@apache.org]
> Sent: Saturday, March 22, 2003 4:38 AM
> To: cocoon-dev@xml.apache.org
> Subject: [RT] pipeline aspects
>
>
> Geoff Howard wrote:
>
> > Obviously, if my
> > application doesn't use any uploads I should disable them in web.xml.
> > But right now, it's all or nothing: I either allow all users to upload
> > _on any page_ (if they create a form that posts to any url in cocoon's
> > space), or I totally disallow uploads.  I've been thinking through
> > enabling configs for resource-based, or even authentication-based
> > restrictions for uploads.  What would others think?
>
> I've been thinking about this for a while and I thought about waiting
> for 2.1 before sending these RT, but I just wanted to introduce them
> briefly before a quick-hack solution is implemented.
>

I printed the rest out to read carefully offline, but in the meantime I have
already started on the "quick-hack" you feared. ;)  I'd love to see a real
solution like you propose post-2.1, but this could work for 2.1.

Actually, this doesn't get
at the restrictions I mentioned above, but to the temp-upload option Vadim
asked for volunteers on.  I've attached a patch which includes the changes
below
to MultiPartParser and a quick change to upload.xsp.  This is meant only to
give the general idea of what would make cleaning up uploads at the end of
service() more feasible.

If no one likes it, I don't want to put any more time in, but if this looks
OK, I can make the changes to CocoonServlet and work on a good way to
provide the configuration option.  Unfortunately, as currently implemented,
autosave-uploads
is boolean and this would introduce a third state.  Options to deal with
that are:
1) modify autosave-uploads from boolean to numeric/string or
2) add temp-uploads boolean.
I'm kind of leaning to 2, but totally open to suggestions.

See also:
http://marc.theaimsgroup.com/?t=104387868200007&r=1&w=2 and
http://marc.theaimsgroup.com/?t=103478709700005&r=1&w=2

Index:
src/java/org/apache/cocoon/components/request/multipart/MultipartParser.java
===================================================================
RCS file:
/home/cvspublic/cocoon-2.1/src/java/org/apache/cocoon/components/request/mul
tipart/MultipartParser.java,v
retrieving revision 1.1
diff -u -r1.1 MultipartParser.java
---
src/java/org/apache/cocoon/components/request/multipart/MultipartParser.java
9 Mar 2003 00:09:10 -0000	1.1
+++
src/java/org/apache/cocoon/components/request/multipart/MultipartParser.java
22 Mar 2003 15:23:43 -0000
@@ -59,6 +59,7 @@
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PushbackInputStream;
+import java.util.ArrayList;
 import java.util.Hashtable;
 import java.util.StringTokenizer;
 import java.util.Vector;
@@ -99,6 +100,10 @@

     /** Field characterEncoding       */
     private String characterEncoding;
+
+	/** Field UPLOAD_ATTRIBUTE - name of request attribute storing
+     * keys to uploaded files placed in the request */
+    private final static String UPLOAD_ATTRIBUTE = "cocoon-uploads";
     /**
      * Constructor, parses given request
      *
@@ -136,7 +141,7 @@
                 new TokenStream(
                         new PushbackInputStream(
                                 new
BufferedInputStream(request.getInputStream()),
-                                MAX_BOUNDARY_SIZE)),
getBoundary(request.getContentType()));
+                                MAX_BOUNDARY_SIZE)),
getBoundary(request.getContentType()), request);
     }

     /**
@@ -148,7 +153,7 @@
      * @throws IOException
      * @throws MultipartException
      */
-    private void parseMultiPart(TokenStream ts, String boundary)
+    private void parseMultiPart(TokenStream ts, String boundary,
HttpServletRequest request)
             throws IOException, MultipartException {

         ts.setBoundary(boundary.getBytes());
@@ -157,7 +162,7 @@

         while (ts.getState() == TokenStream.STATE_NEXTPART) {
             ts.nextPart();
-            parsePart(ts);
+            parsePart(ts, request);
         }

         if (ts.getState() != TokenStream.STATE_ENDMULTIPART) {    // sanity
check
@@ -173,7 +178,7 @@
      * @throws IOException
      * @throws MultipartException
      */
-    private void parsePart(TokenStream ts)
+    private void parsePart(TokenStream ts, HttpServletRequest request)
             throws IOException, MultipartException {

         Hashtable headers = new Hashtable();
@@ -181,7 +186,7 @@
         try {
             if (headers.containsKey("filename")) {
 		        if (!"".equals(headers.get("filename"))) {
-                	parseFilePart(ts, headers);
+                	parseFilePart(ts, headers, request);
 		        } else {
         			// IE6 sends an empty part with filename="" for
         			// empty upload fields. Just parse away the part
@@ -198,7 +203,7 @@
             else if (((String)
headers.get("content-disposition")).toLowerCase()
                     .indexOf("multipart") > -1) {
                 parseMultiPart(new TokenStream(ts, MAX_BOUNDARY_SIZE),
-                        "--" + (String) headers.get("boundary"));
+                        "--" + (String) headers.get("boundary"), request);
                 ts.read();    // read past boundary
             } else {
                 throw new MultipartException("Unknown part type");
@@ -219,7 +224,7 @@
      * @throws IOException
      * @throws MultipartException
      */
-    private void parseFilePart(TokenStream in, Hashtable headers)
+    private void parseFilePart(TokenStream in, Hashtable headers,
HttpServletRequest request)
             throws IOException, MultipartException {

         byte[] buf = new byte[FILE_BUFFER_SIZE];
@@ -241,6 +246,7 @@

             if (file.exists()) {
                 if (!allowOverwrite) {
+                    // FIXME: race condition if simultaneous threads upload
a file of the same name
                     if (silentlyRename) {
                         int c = 0;

@@ -272,6 +278,13 @@
         } else {
             put(headers.get("name"), new FilePartFile(headers, file));
         }
+
+        ArrayList uploads =
(ArrayList)request.getAttribute(UPLOAD_ATTRIBUTE);;
+        if (uploads == null) {
+            uploads = new ArrayList();
+        }
+        uploads.add(headers.get("name"));
+        request.setAttribute(UPLOAD_ATTRIBUTE,uploads);
     }

     /**