You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@turbine.apache.org by Brian Lawler <br...@tribenetwork.com> on 2004/04/06 03:50:31 UTC

A little change to the Parameter Parser that others may find useful...

Hey guys-

I don't know what the normal protocol is for low priority features, but  
here goes...

At tribe, we have a need to scrub all of the strings that are coming  
from our forms to make sure that end users are not entering in  
malicious HTML in the free-form text areas.  The way we have done this  
is to refactor the setRequest processing to call a scrub() method.   
Scrub is protected and by default does nothing, but our implementation  
overrides this method with a simple HTML scrubbing of all the strings  
passed in.

Again, I don't know how you guys manage tiny changes like this - if it  
is considered polluting the codebase or not.  If anyone has done this a  
different way I would be interested in hearing about it.  At this  
point, it is my goal to keep our local turbine code as close to the 2.3  
repository as possible.  Let me know what you think...

-Brian

Index:  
src/java/org/apache/turbine/util/parser/DefaultParameterParser.java
===================================================================
RCS file:  
/home/cvspublic/jakarta-turbine-2/src/java/org/apache/turbine/util/ 
parser/DefaultParameterParser.java,v
retrieving revision 1.20.2.1
diff -u -r1.20.2.1 DefaultParameterParser.java
--- src/java/org/apache/turbine/util/parser/DefaultParameterParser.java  
27 Feb 2004 10:34:24 -0000      1.20.2.1
+++ src/java/org/apache/turbine/util/parser/DefaultParameterParser.java  
6 Apr 2004 01:47:45 -0000
@@ -192,8 +192,7 @@
               names.hasMoreElements();)
          {
              tmp = (String) names.nextElement();
-            add(convert(tmp),
-                    request.getParameterValues(tmp));
+            add(convert(tmp), scrub(request.getParameterValues(tmp)));
          }

          // Also cache any pathinfo variables that are passed around as
@@ -337,5 +336,16 @@
                      + name + ") is not an instance of FileItem[]", e);
              return null;
          }
+    }
+
+   /**
+    * The scrub method is used by setRequest() to do any  
pre-processing needed
+    * on inbound strings.  The default implementation is to do  
nothing, but
+    * applications may overide this method to add any specific  
behavior they
+    * need in their business rules (such as HTML tag scrubbing).
+    */
+    protected String[] scrub(String vals[])
+    {
+        return vals;
      }
  }


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


RE: A little change to the Parameter Parser that others may find useful...

Posted by Eric Pugh <ep...@upstate.com>.
Talk to Peter (Peter Courcoux [peter@courcoux.biz]), he has implemented
something to do the same kind of thing as part of his
enhancement/replacement for Intake..

To be honest, I think that if you also provided the scrubber, then the
change would be more useful, or maybe provide a different implementation of
parameter parser?

At any rate, Peter has talked about contributing the work he has done, so
maybe between the two of you, you can come up with the best implementation
to be added to Turbine?

The process you are following is exactly right, however something I always
like to see on new contributions is a unit test exercising the new code..
It shows a depth to the testing and makes it easier for people in the future
to understand how the code functions.

Eric

> -----Original Message-----
> From: Brian Lawler [mailto:brian@tribenetwork.com]
> Sent: Monday, April 05, 2004 9:51 PM
> To: Turbine Developers List
> Subject: A little change to the Parameter Parser that others may find
> useful...
>
>
> Hey guys-
>
> I don't know what the normal protocol is for low priority features, but
> here goes...
>
> At tribe, we have a need to scrub all of the strings that are coming
> from our forms to make sure that end users are not entering in
> malicious HTML in the free-form text areas.  The way we have done this
> is to refactor the setRequest processing to call a scrub() method.
> Scrub is protected and by default does nothing, but our implementation
> overrides this method with a simple HTML scrubbing of all the strings
> passed in.
>
> Again, I don't know how you guys manage tiny changes like this - if it
> is considered polluting the codebase or not.  If anyone has done this a
> different way I would be interested in hearing about it.  At this
> point, it is my goal to keep our local turbine code as close to the 2.3
> repository as possible.  Let me know what you think...
>
> -Brian
>
> Index:
> src/java/org/apache/turbine/util/parser/DefaultParameterParser.java
> ===================================================================
> RCS file:
> /home/cvspublic/jakarta-turbine-2/src/java/org/apache/turbine/util/
> parser/DefaultParameterParser.java,v
> retrieving revision 1.20.2.1
> diff -u -r1.20.2.1 DefaultParameterParser.java
> --- src/java/org/apache/turbine/util/parser/DefaultParameterParser.java
> 27 Feb 2004 10:34:24 -0000      1.20.2.1
> +++ src/java/org/apache/turbine/util/parser/DefaultParameterParser.java
> 6 Apr 2004 01:47:45 -0000
> @@ -192,8 +192,7 @@
>                names.hasMoreElements();)
>           {
>               tmp = (String) names.nextElement();
> -            add(convert(tmp),
> -                    request.getParameterValues(tmp));
> +            add(convert(tmp), scrub(request.getParameterValues(tmp)));
>           }
>
>           // Also cache any pathinfo variables that are passed around as
> @@ -337,5 +336,16 @@
>                       + name + ") is not an instance of FileItem[]", e);
>               return null;
>           }
> +    }
> +
> +   /**
> +    * The scrub method is used by setRequest() to do any
> pre-processing needed
> +    * on inbound strings.  The default implementation is to do
> nothing, but
> +    * applications may overide this method to add any specific
> behavior they
> +    * need in their business rules (such as HTML tag scrubbing).
> +    */
> +    protected String[] scrub(String vals[])
> +    {
> +        return vals;
>       }
>   }
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: turbine-dev-help@jakarta.apache.org


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