You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2011/11/07 11:46:15 UTC

svn commit: r1198696 - in /tomcat/trunk/java/org/apache: catalina/Globals.java catalina/connector/Request.java catalina/filters/FailedRequestFilter.java tomcat/util/http/Parameters.java

Author: kkolinko
Date: Mon Nov  7 10:46:14 2011
New Revision: 1198696

URL: http://svn.apache.org/viewvc?rev=1198696&view=rev
Log:
Introduce new request attribute to be used to mark request if there was a failure during parameter parsing,
and a Filter that triggers parameter parsing and rejects requests marked with that attribute.

Added:
    tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java   (with props)
Modified:
    tomcat/trunk/java/org/apache/catalina/Globals.java
    tomcat/trunk/java/org/apache/catalina/connector/Request.java
    tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java

Modified: tomcat/trunk/java/org/apache/catalina/Globals.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/Globals.java?rev=1198696&r1=1198695&r2=1198696&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/Globals.java (original)
+++ tomcat/trunk/java/org/apache/catalina/Globals.java Mon Nov  7 10:46:14 2011
@@ -226,6 +226,17 @@ public final class Globals {
 
 
     /**
+     * The request attribute that is set to {@code Boolean.TRUE} if some request
+     * parameters have been ignored during request parameters parsing. It can
+     * happen, for example, if there is a limit on the total count of parseable
+     * parameters, or if parameter cannot be decoded, or any other error
+     * happened during parameter parsing.
+     */
+    public static final String PARAMETER_PARSE_FAILED_ATTR =
+        "org.apache.catalina.parameter_parse_failed";
+
+
+    /**
      * The master flag which controls strict servlet specification
      * compliance.
      */

Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?rev=1198696&r1=1198695&r2=1198696&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Mon Nov  7 10:46:14 2011
@@ -2383,6 +2383,12 @@ public class Request
         }
     }
 
+    private void checkParameterParseFailed() {
+        if (getCoyoteRequest().getParameters().isParseFailed()) {
+            setAttribute(Globals.PARAMETER_PARSE_FAILED_ATTR, Boolean.TRUE);
+        }
+    }
+
     public void cometClose() {
         coyoteRequest.action(ActionCode.COMET_CLOSE,getEvent());
         setComet(false);
@@ -2487,109 +2493,117 @@ public class Request
 
         Parameters parameters = coyoteRequest.getParameters();
 
-        File location;
-        String locationStr = mce.getLocation();
-        if (locationStr == null || locationStr.length() == 0) {
-            location = ((File) context.getServletContext().getAttribute(
-                    ServletContext.TEMPDIR));
-        } else {
-            location = new File(locationStr);
-        }
-
-        if (!location.isAbsolute() || !location.isDirectory()) {
-            partsParseException = new IOException(
-                    sm.getString("coyoteRequest.uploadLocationInvalid",
-                            location));
-            return;
-        }
-
-        // Create a new file upload handler
-        DiskFileItemFactory factory = new DiskFileItemFactory();
+        boolean success = false;
         try {
-            factory.setRepository(location.getCanonicalFile());
-        } catch (IOException ioe) {
-            partsParseException = ioe;
-            return;
-        }
-        factory.setSizeThreshold(mce.getFileSizeThreshold());
+            File location;
+            String locationStr = mce.getLocation();
+            if (locationStr == null || locationStr.length() == 0) {
+                location = ((File) context.getServletContext().getAttribute(
+                        ServletContext.TEMPDIR));
+            } else {
+                location = new File(locationStr);
+            }
 
-        ServletFileUpload upload = new ServletFileUpload();
-        upload.setFileItemFactory(factory);
-        upload.setFileSizeMax(mce.getMaxFileSize());
-        upload.setSizeMax(mce.getMaxRequestSize());
+            if (!location.isAbsolute() || !location.isDirectory()) {
+                partsParseException = new IOException(
+                        sm.getString("coyoteRequest.uploadLocationInvalid",
+                                location));
+                return;
+            }
 
-        parts = new ArrayList<Part>();
-        try {
-            List<FileItem> items = upload.parseRequest(this);
-            int maxPostSize = getConnector().getMaxPostSize();
-            int postSize = 0;
-            String enc = getCharacterEncoding();
-            Charset charset = null;
-            if (enc != null) {
-                try {
-                    charset = B2CConverter.getCharset(enc);
-                } catch (UnsupportedEncodingException e) {
-                    // Ignore
-                }
+
+            // Create a new file upload handler
+            DiskFileItemFactory factory = new DiskFileItemFactory();
+            try {
+                factory.setRepository(location.getCanonicalFile());
+            } catch (IOException ioe) {
+                partsParseException = ioe;
+                return;
             }
-            for (FileItem item : items) {
-                ApplicationPart part = new ApplicationPart(item, mce);
-                parts.add(part);
-                if (part.getFilename() == null) {
-                    String name = part.getName();
-                    String value = null;
+            factory.setSizeThreshold(mce.getFileSizeThreshold());
+
+            ServletFileUpload upload = new ServletFileUpload();
+            upload.setFileItemFactory(factory);
+            upload.setFileSizeMax(mce.getMaxFileSize());
+            upload.setSizeMax(mce.getMaxRequestSize());
+
+            parts = new ArrayList<Part>();
+            try {
+                List<FileItem> items = upload.parseRequest(this);
+                int maxPostSize = getConnector().getMaxPostSize();
+                int postSize = 0;
+                String enc = getCharacterEncoding();
+                Charset charset = null;
+                if (enc != null) {
                     try {
-                        String encoding = parameters.getEncoding();
-                        if (encoding == null) {
-                            encoding = Parameters.DEFAULT_ENCODING;
-                        }
-                        value = part.getString(encoding);
-                    } catch (UnsupportedEncodingException uee) {
-                        try {
-                            value = part.getString(Parameters.DEFAULT_ENCODING);
-                        } catch (UnsupportedEncodingException e) {
-                            // Should not be possible
-                        }
+                        charset = B2CConverter.getCharset(enc);
+                    } catch (UnsupportedEncodingException e) {
+                        // Ignore
                     }
-                    if (maxPostSize > 0) {
-                        // Have to calculate equivalent size. Not completely
-                        // accurate but close enough.
-                        if (charset == null) {
-                            // Name length
-                            postSize += name.getBytes().length;
-                        } else {
-                            postSize += name.getBytes(charset).length;
+                }
+                for (FileItem item : items) {
+                    ApplicationPart part = new ApplicationPart(item, mce);
+                    parts.add(part);
+                    if (part.getFilename() == null) {
+                        String name = part.getName();
+                        String value = null;
+                        try {
+                            String encoding = parameters.getEncoding();
+                            if (encoding == null) {
+                                encoding = Parameters.DEFAULT_ENCODING;
+                            }
+                            value = part.getString(encoding);
+                        } catch (UnsupportedEncodingException uee) {
+                            try {
+                                value = part.getString(Parameters.DEFAULT_ENCODING);
+                            } catch (UnsupportedEncodingException e) {
+                                // Should not be possible
+                            }
                         }
-                        if (value != null) {
-                            // Equals sign
+                        if (maxPostSize > 0) {
+                            // Have to calculate equivalent size. Not completely
+                            // accurate but close enough.
+                            if (charset == null) {
+                                // Name length
+                                postSize += name.getBytes().length;
+                            } else {
+                                postSize += name.getBytes(charset).length;
+                            }
+                            if (value != null) {
+                                // Equals sign
+                                postSize++;
+                                // Value length
+                                postSize += part.getSize();
+                            }
+                            // Value separator
                             postSize++;
-                            // Value length
-                            postSize += part.getSize();
-                        }
-                        // Value separator
-                        postSize++;
-                        if (postSize > maxPostSize) {
-                            throw new IllegalStateException(sm.getString(
-                                    "coyoteRequest.maxPostSizeExceeded"));
+                            if (postSize > maxPostSize) {
+                                throw new IllegalStateException(sm.getString(
+                                        "coyoteRequest.maxPostSizeExceeded"));
+                            }
                         }
+                        parameters.addParameter(name, value);
                     }
-                    parameters.addParameter(name, value);
                 }
-            }
 
-        } catch (InvalidContentTypeException e) {
-            partsParseException = new ServletException(e);
-        } catch (FileUploadBase.SizeException e) {
-            checkSwallowInput();
-            partsParseException = new IllegalStateException(e);
-        } catch (FileUploadException e) {
-            partsParseException = new IOException(e);
-        } catch (IllegalStateException e) {
-            checkSwallowInput();
-            partsParseException = e;
+                success = true;
+            } catch (InvalidContentTypeException e) {
+                partsParseException = new ServletException(e);
+            } catch (FileUploadBase.SizeException e) {
+                checkSwallowInput();
+                partsParseException = new IllegalStateException(e);
+            } catch (FileUploadException e) {
+                partsParseException = new IOException(e);
+            } catch (IllegalStateException e) {
+                checkSwallowInput();
+                partsParseException = e;
+            }
+        } finally {
+            if (partsParseException != null || !success) {
+                parameters.setParseFailed(true);
+            }
+            checkParameterParseFailed();
         }
-
-        return;
     }
 
 
@@ -2774,108 +2788,120 @@ public class Request
         parametersParsed = true;
 
         Parameters parameters = coyoteRequest.getParameters();
-        // Set this every time in case limit has been changed via JMX
-        parameters.setLimit(getConnector().getMaxParameterCount());
+        boolean success = false;
+        try {
+            // Set this every time in case limit has been changed via JMX
+            parameters.setLimit(getConnector().getMaxParameterCount());
 
-        // getCharacterEncoding() may have been overridden to search for
-        // hidden form field containing request encoding
-        String enc = getCharacterEncoding();
-
-        boolean useBodyEncodingForURI = connector.getUseBodyEncodingForURI();
-        if (enc != null) {
-            parameters.setEncoding(enc);
-            if (useBodyEncodingForURI) {
-                parameters.setQueryStringEncoding(enc);
-            }
-        } else {
-            parameters.setEncoding
-                (org.apache.coyote.Constants.DEFAULT_CHARACTER_ENCODING);
-            if (useBodyEncodingForURI) {
-                parameters.setQueryStringEncoding
+            // getCharacterEncoding() may have been overridden to search for
+            // hidden form field containing request encoding
+            String enc = getCharacterEncoding();
+
+            boolean useBodyEncodingForURI = connector.getUseBodyEncodingForURI();
+            if (enc != null) {
+                parameters.setEncoding(enc);
+                if (useBodyEncodingForURI) {
+                    parameters.setQueryStringEncoding(enc);
+                }
+            } else {
+                parameters.setEncoding
                     (org.apache.coyote.Constants.DEFAULT_CHARACTER_ENCODING);
+                if (useBodyEncodingForURI) {
+                    parameters.setQueryStringEncoding
+                        (org.apache.coyote.Constants.DEFAULT_CHARACTER_ENCODING);
+                }
             }
-        }
 
-        parameters.handleQueryParameters();
+            parameters.handleQueryParameters();
 
-        if (usingInputStream || usingReader) {
-            return;
-        }
+            if (usingInputStream || usingReader) {
+                success = true;
+                return;
+            }
 
-        if( !getConnector().isParseBodyMethod(getMethod()) ) {
-            return;
-        }
+            if( !getConnector().isParseBodyMethod(getMethod()) ) {
+                success = true;
+                return;
+            }
 
-        String contentType = getContentType();
-        if (contentType == null) {
-            contentType = "";
-        }
-        int semicolon = contentType.indexOf(';');
-        if (semicolon >= 0) {
-            contentType = contentType.substring(0, semicolon).trim();
-        } else {
-            contentType = contentType.trim();
-        }
+            String contentType = getContentType();
+            if (contentType == null) {
+                contentType = "";
+            }
+            int semicolon = contentType.indexOf(';');
+            if (semicolon >= 0) {
+                contentType = contentType.substring(0, semicolon).trim();
+            } else {
+                contentType = contentType.trim();
+            }
 
-        if ("multipart/form-data".equals(contentType)) {
-            parseParts();
-            return;
-        }
+            if ("multipart/form-data".equals(contentType)) {
+                parseParts();
+                success = true;
+                return;
+            }
 
-        if (!("application/x-www-form-urlencoded".equals(contentType))) {
-            return;
-        }
+            if (!("application/x-www-form-urlencoded".equals(contentType))) {
+                success = true;
+                return;
+            }
 
-        int len = getContentLength();
+            int len = getContentLength();
 
-        if (len > 0) {
-            int maxPostSize = connector.getMaxPostSize();
-            if ((maxPostSize > 0) && (len > maxPostSize)) {
-                if (context.getLogger().isDebugEnabled()) {
-                    context.getLogger().debug(
-                            sm.getString("coyoteRequest.postTooLarge"));
+            if (len > 0) {
+                int maxPostSize = connector.getMaxPostSize();
+                if ((maxPostSize > 0) && (len > maxPostSize)) {
+                    if (context.getLogger().isDebugEnabled()) {
+                        context.getLogger().debug(
+                                sm.getString("coyoteRequest.postTooLarge"));
+                    }
+                    checkSwallowInput();
+                    return;
                 }
-                checkSwallowInput();
-                return;
-            }
-            byte[] formData = null;
-            if (len < CACHED_POST_LEN) {
-                if (postData == null) {
-                    postData = new byte[CACHED_POST_LEN];
+                byte[] formData = null;
+                if (len < CACHED_POST_LEN) {
+                    if (postData == null) {
+                        postData = new byte[CACHED_POST_LEN];
+                    }
+                    formData = postData;
+                } else {
+                    formData = new byte[len];
                 }
-                formData = postData;
-            } else {
-                formData = new byte[len];
-            }
-            try {
-                if (readPostBody(formData, len) != len) {
+                try {
+                    if (readPostBody(formData, len) != len) {
+                        return;
+                    }
+                } catch (IOException e) {
+                    // Client disconnect
+                    if (context.getLogger().isDebugEnabled()) {
+                        context.getLogger().debug(
+                                sm.getString("coyoteRequest.parseParameters"), e);
+                    }
                     return;
                 }
-            } catch (IOException e) {
-                // Client disconnect
-                if (context.getLogger().isDebugEnabled()) {
-                    context.getLogger().debug(
-                            sm.getString("coyoteRequest.parseParameters"), e);
+                parameters.processParameters(formData, 0, len);
+            } else if ("chunked".equalsIgnoreCase(
+                    coyoteRequest.getHeader("transfer-encoding"))) {
+                byte[] formData = null;
+                try {
+                    formData = readChunkedPostBody();
+                } catch (IOException e) {
+                    // Client disconnect
+                    if (context.getLogger().isDebugEnabled()) {
+                        context.getLogger().debug(
+                                sm.getString("coyoteRequest.parseParameters"), e);
+                    }
+                    return;
                 }
-                return;
-            }
-            parameters.processParameters(formData, 0, len);
-        } else if ("chunked".equalsIgnoreCase(
-                coyoteRequest.getHeader("transfer-encoding"))) {
-            byte[] formData = null;
-            try {
-                formData = readChunkedPostBody();
-            } catch (IOException e) {
-                // Client disconnect
-                if (context.getLogger().isDebugEnabled()) {
-                    context.getLogger().debug(
-                            sm.getString("coyoteRequest.parseParameters"), e);
+                if (formData != null) {
+                    parameters.processParameters(formData, 0, formData.length);
                 }
-                return;
             }
-            if (formData != null) {
-                parameters.processParameters(formData, 0, formData.length);
+        } finally {
+            if (!success) {
+                parameters.setParseFailed(true);
             }
+            checkParameterParseFailed();
         }
 
     }

Added: tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java?rev=1198696&view=auto
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java (added)
+++ tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java Mon Nov  7 10:46:14 2011
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.filters;
+
+import java.io.IOException;
+
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.catalina.Globals;
+import org.apache.catalina.comet.CometEvent;
+import org.apache.catalina.comet.CometFilter;
+import org.apache.catalina.comet.CometFilterChain;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+
+/**
+ * Filter that will reject requests if there was a failure during parameter
+ * parsing. This filter can be used to ensure that none parameter values
+ * submitted by client are lost.
+ *
+ * <p>
+ * Note that it has side effect that it triggers parameter parsing and thus
+ * consumes the body for POST requests, so use it only with addresses that do not
+ * use <code>request.getInputStream()</code> and
+ * <code>request.getReader()</code>.
+ */
+public class FailedRequestFilter extends FilterBase implements CometFilter {
+
+    private static final Log log = LogFactory.getLog(FailedRequestFilter.class);
+
+    @Override
+    protected Log getLogger() {
+        return log;
+    }
+
+    @Override
+    public void doFilter(ServletRequest request, ServletResponse response,
+            FilterChain chain) throws IOException, ServletException {
+        if (!isGoodRequest(request)) {
+            ((HttpServletResponse) response)
+                    .sendError(HttpServletResponse.SC_BAD_REQUEST);
+            return;
+        }
+        chain.doFilter(request, response);
+    }
+
+    @Override
+    public void doFilterEvent(CometEvent event, CometFilterChain chain)
+            throws IOException, ServletException {
+        if (event.getEventType() == CometEvent.EventType.BEGIN
+                && !isGoodRequest(event.getHttpServletRequest())) {
+            event.getHttpServletResponse().sendError(
+                    HttpServletResponse.SC_BAD_REQUEST);
+            event.close();
+            return;
+        }
+        chain.doFilterEvent(event);
+    }
+
+    private boolean isGoodRequest(ServletRequest request) {
+        // Trigger parsing of parameters
+        request.getParameter("none");
+        // Detect failure
+        if (request.getAttribute(Globals.PARAMETER_PARSE_FAILED_ATTR) != null) {
+            return false;
+        }
+        return true;
+    }
+
+    @Override
+    protected boolean isConfigProblemFatal() {
+        return true;
+    }
+
+}

Propchange: tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java?rev=1198696&r1=1198695&r2=1198696&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java Mon Nov  7 10:46:14 2011
@@ -59,6 +59,12 @@ public final class Parameters {
     private int limit = -1;
     private int parameterCount = 0;
 
+    /**
+     * Is set to <code>true</code> if there were failures during parameter
+     * parsing.
+     */
+    private boolean parseFailed = false;
+
     public Parameters() {
         // NO-OP
     }
@@ -89,12 +95,21 @@ public final class Parameters {
         }
     }
 
+    public boolean isParseFailed() {
+        return parseFailed;
+    }
+
+    public void setParseFailed(boolean parseFailed) {
+        this.parseFailed = parseFailed;
+    }
+
     public void recycle() {
         parameterCount = 0;
         paramHashValues.clear();
         didQueryParameters=false;
         encoding=null;
         decodedQuery.recycle();
+        parseFailed = false;
     }
 
     // -------------------- Data access --------------------
@@ -168,6 +183,7 @@ public final class Parameters {
         if (limit > -1 && parameterCount > limit) {
             // Processing this parameter will push us over the limit. ISE is
             // what Request.parseParts() uses for requests that are too big
+            parseFailed = true;
             throw new IllegalStateException(sm.getString(
                     "parameters.maxCountFail", Integer.valueOf(limit)));
         }
@@ -296,6 +312,7 @@ public final class Parameters {
                                 null));
                     }
                 }
+                parseFailed = true;
                 continue;
                 // invalid chunk - it's better to ignore
             }
@@ -337,10 +354,12 @@ public final class Parameters {
                 } catch (IllegalStateException ise) {
                     // Hitting limit stops processing further params but does
                     // not cause request to fail.
+                    parseFailed = true;
                     log.warn(ise.getMessage());
                     break;
                 }
             } catch (IOException e) {
+                parseFailed = true;
                 decodeFailCount++;
                 if (decodeFailCount == 1 || log.isDebugEnabled()) {
                     if (log.isDebugEnabled()) {



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


Re: svn commit: r1198696 - in /tomcat/trunk/java/org/apache: catalina/Globals.java catalina/connector/Request.java catalina/filters/FailedRequestFilter.java tomcat/util/http/Parameters.java

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/11/7 Rainer Jung <ra...@kippdata.de>:
> Hi Konstantin,
>
> On 07.11.2011 02:46, kkolinko@apache.org wrote:
>> Author: kkolinko
>> Date: Mon Nov  7 10:46:14 2011
>> New Revision: 1198696
>>
>> URL: http://svn.apache.org/viewvc?rev=1198696&view=rev
>> Log:
>> Introduce new request attribute to be used to mark request if there was a failure during parameter parsing,
>> and a Filter that triggers parameter parsing and rejects requests marked with that attribute.
>>
>> Added:
>>     tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java   (with props)
>> Modified:
>>     tomcat/trunk/java/org/apache/catalina/Globals.java
>>     tomcat/trunk/java/org/apache/catalina/connector/Request.java
>>     tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java
>>
>>
>>      /**
>> +     * The request attribute that is set to {@code Boolean.TRUE} if some request
>> +     * parameters have been ignored during request parameters parsing. It can
>> +     * happen, for example, if there is a limit on the total count of parseable
>> +     * parameters, or if parameter cannot be decoded, or any other error
>> +     * happened during parameter parsing.
>> +     */
>> +    public static final String PARAMETER_PARSE_FAILED_ATTR =
>> +        "org.apache.catalina.parameter_parse_failed";
>
> I don't now if we ever have to make the new request attribute available
> in coyote request, but in o.a.c.connector.Request there is code that
> only passes attributes down to the coyote request if the attribute name
> starts with "org.apache.tomcat". See removeAttribute() and setAttribute():
>
> 1413         // Pass special attributes to the native layer
> 1414         if (name.startsWith("org.apache.tomcat.")) {
> 1415             coyoteRequest.getAttributes().remove(name);
> 1416         }
> ...
> 1520         // Pass special attributes to the native layer
> 1521         if (name.startsWith("org.apache.tomcat.")) {
> 1522             coyoteRequest.setAttribute(name, value);
> 1523         }
>
> In fact this use of "org.apache.tomcat." could also another Global constant.
>

Thank you for the comment.

I agree "org.apache.tomcat." could be a constant.
Need to think of a good name like COYOTE_ATTR_PREFIX.
The "Pass ... to the native layer" comment is understandable,
but seems outdated with Nio implementation there besides Apr one.

If there isn't a constant, there could be a comment in Globals
mentioning the prefix.


Regarding PARAMETER_PARSE_FAILED_ATTR, its value is present
in the "native layer" as Parameters.isParseFailed(),
and the attribute is used only to pass this information up to Servlets.

I delegate the decision what to do to Valves/Filters/Servlets that
check the attribute. I do not see anything that can happen at the
"native layer".


Instead of Request#checkParameterParseFailed() there could be
alternative implementation as a new "if(name.equals(...))" branch in
Request#getAttribute().

Side effects are that the new attribute is listed in
Request.getAttributeNames() enumeration (unlike other special
attributes that are omitted there) and can be changed by
Request#setAttribute(), Request#removeAttribute().


Best regards,
Konstantin Kolinko

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


Re: svn commit: r1198696 - in /tomcat/trunk/java/org/apache: catalina/Globals.java catalina/connector/Request.java catalina/filters/FailedRequestFilter.java tomcat/util/http/Parameters.java

Posted by Rainer Jung <ra...@kippdata.de>.
Hi Konstantin,

On 07.11.2011 02:46, kkolinko@apache.org wrote:
> Author: kkolinko
> Date: Mon Nov  7 10:46:14 2011
> New Revision: 1198696
> 
> URL: http://svn.apache.org/viewvc?rev=1198696&view=rev
> Log:
> Introduce new request attribute to be used to mark request if there was a failure during parameter parsing,
> and a Filter that triggers parameter parsing and rejects requests marked with that attribute.
> 
> Added:
>     tomcat/trunk/java/org/apache/catalina/filters/FailedRequestFilter.java   (with props)
> Modified:
>     tomcat/trunk/java/org/apache/catalina/Globals.java
>     tomcat/trunk/java/org/apache/catalina/connector/Request.java
>     tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java
> 
> Modified: tomcat/trunk/java/org/apache/catalina/Globals.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/Globals.java?rev=1198696&r1=1198695&r2=1198696&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/Globals.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/Globals.java Mon Nov  7 10:46:14 2011
> @@ -226,6 +226,17 @@ public final class Globals {
>  
>  
>      /**
> +     * The request attribute that is set to {@code Boolean.TRUE} if some request
> +     * parameters have been ignored during request parameters parsing. It can
> +     * happen, for example, if there is a limit on the total count of parseable
> +     * parameters, or if parameter cannot be decoded, or any other error
> +     * happened during parameter parsing.
> +     */
> +    public static final String PARAMETER_PARSE_FAILED_ATTR =
> +        "org.apache.catalina.parameter_parse_failed";

I don't now if we ever have to make the new request attribute available
in coyote request, but in o.a.c.connector.Request there is code that
only passes attributes down to the coyote request if the attribute name
starts with "org.apache.tomcat". See removeAttribute() and setAttribute():

1413         // Pass special attributes to the native layer
1414         if (name.startsWith("org.apache.tomcat.")) {
1415             coyoteRequest.getAttributes().remove(name);
1416         }
...
1520         // Pass special attributes to the native layer
1521         if (name.startsWith("org.apache.tomcat.")) {
1522             coyoteRequest.setAttribute(name, value);
1523         }

In fact this use of "org.apache.tomcat." could also another Global constant.

Regards,

Rainer

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