You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2014/09/03 20:53:56 UTC

svn commit: r1622314 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/connector/ java/org/apache/tomcat/util/http/parser/ test/org/apache/catalina/connector/ test/org/apache/tomcat/util/http/parser/ webapps/docs/

Author: markt
Date: Wed Sep  3 18:53:56 2014
New Revision: 1622314

URL: http://svn.apache.org/r1622314
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56848
Revert r1622303. It depends on a Java 7 API.

Removed:
    tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/parser/AcceptLanguage.java
    tomcat/tc7.0.x/trunk/test/org/apache/tomcat/util/http/parser/TestAcceptLanguage.java
Modified:
    tomcat/tc7.0.x/trunk/   (props changed)
    tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Request.java
    tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
    tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/parser/MediaType.java
    tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestRequest.java
    tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TesterRequest.java
    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
  Reverse-merged /tomcat/trunk:r1618166

Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Request.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Request.java?rev=1622314&r1=1622313&r2=1622314&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Request.java Wed Sep  3 18:53:56 2014
@@ -21,7 +21,6 @@ import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.StringReader;
 import java.io.UnsupportedEncodingException;
 import java.lang.reflect.InvocationTargetException;
 import java.nio.charset.Charset;
@@ -75,6 +74,7 @@ import org.apache.catalina.core.Applicat
 import org.apache.catalina.core.AsyncContextImpl;
 import org.apache.catalina.realm.GenericPrincipal;
 import org.apache.catalina.util.ParameterMap;
+import org.apache.catalina.util.StringParser;
 import org.apache.coyote.ActionCode;
 import org.apache.coyote.http11.upgrade.servlet31.HttpUpgradeHandler;
 import org.apache.juli.logging.Log;
@@ -95,7 +95,6 @@ import org.apache.tomcat.util.http.fileu
 import org.apache.tomcat.util.http.fileupload.servlet.ServletFileUpload;
 import org.apache.tomcat.util.http.fileupload.servlet.ServletRequestContext;
 import org.apache.tomcat.util.http.mapper.MappingData;
-import org.apache.tomcat.util.http.parser.AcceptLanguage;
 import org.apache.tomcat.util.res.StringManager;
 import org.ietf.jgss.GSSCredential;
 import org.ietf.jgss.GSSException;
@@ -376,6 +375,12 @@ public class Request
 
 
     /**
+     * The string parser we will use for parsing request lines.
+     */
+    private final StringParser parser = new StringParser();
+
+
+    /**
      * Local port
      */
     protected int localPort = -1;
@@ -3249,24 +3254,99 @@ public class Request
      */
     protected void parseLocalesHeader(String value, TreeMap<Double, ArrayList<Locale>> locales) {
 
-        List<AcceptLanguage> acceptLanguages;
-        try {
-            acceptLanguages = AcceptLanguage.parse(new StringReader(value));
-        } catch (IOException e) {
-            // Mal-formed headers are ignore. Do the same in the unlikely event
-            // of an IOException.
-            return;
+        // Preprocess the value to remove all whitespace
+        int white = value.indexOf(' ');
+        if (white < 0) {
+            white = value.indexOf('\t');
+        }
+        if (white >= 0) {
+            StringBuilder sb = new StringBuilder();
+            int len = value.length();
+            for (int i = 0; i < len; i++) {
+                char ch = value.charAt(i);
+                if ((ch != ' ') && (ch != '\t')) {
+                    sb.append(ch);
+                }
+            }
+            parser.setString(sb.toString());
+        } else {
+            parser.setString(value);
         }
 
-        for (AcceptLanguage acceptLanguage : acceptLanguages) {
+        // Process each comma-delimited language specification
+        int length = parser.getLength();
+        while (true) {
+
+            // Extract the next comma-delimited entry
+            int start = parser.getIndex();
+            if (start >= length) {
+                break;
+            }
+            int end = parser.findChar(',');
+            String entry = parser.extract(start, end).trim();
+            parser.advance();   // For the following entry
+
+            // Extract the quality factor for this entry
+            double quality = 1.0;
+            int semi = entry.indexOf(";q=");
+            if (semi >= 0) {
+                try {
+                    String strQuality = entry.substring(semi + 3);
+                    if (strQuality.length() <= 5) {
+                        quality = Double.parseDouble(strQuality);
+                    } else {
+                        quality = 0.0;
+                    }
+                } catch (NumberFormatException e) {
+                    quality = 0.0;
+                }
+                entry = entry.substring(0, semi);
+            }
+
+            // Skip entries we are not going to keep track of
+            if (quality < 0.00005)
+             {
+                continue;       // Zero (or effectively zero) quality factors
+            }
+            if ("*".equals(entry))
+             {
+                continue;       // FIXME - "*" entries are not handled
+            }
+
+            // Extract the language and country for this entry
+            String language = null;
+            String country = null;
+            String variant = null;
+            int dash = entry.indexOf('-');
+            if (dash < 0) {
+                language = entry;
+                country = "";
+                variant = "";
+            } else {
+                language = entry.substring(0, dash);
+                country = entry.substring(dash + 1);
+                int vDash = country.indexOf('-');
+                if (vDash > 0) {
+                    String cTemp = country.substring(0, vDash);
+                    variant = country.substring(vDash + 1);
+                    country = cTemp;
+                } else {
+                    variant = "";
+                }
+            }
+            if (!isAlpha(language) || !isAlpha(country) || !isAlpha(variant)) {
+                continue;
+            }
+
             // Add a new Locale to the list of Locales for this quality level
-            Double key = new Double(-acceptLanguage.getQuality());  // Reverse the order
+            Locale locale = new Locale(language, country, variant);
+            Double key = new Double(-quality);  // Reverse the order
             ArrayList<Locale> values = locales.get(key);
             if (values == null) {
                 values = new ArrayList<Locale>();
                 locales.put(key, values);
             }
-            values.add(acceptLanguage.getLocale());
+            values.add(locale);
         }
     }
 

Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java?rev=1622314&r1=1622313&r2=1622314&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java Wed Sep  3 18:53:56 2014
@@ -119,7 +119,7 @@ public class HttpParser {
 
         Map<String,String> result = new HashMap<String,String>();
 
-        if (skipConstant(input, "Digest") != SkipResult.FOUND) {
+        if (skipConstant(input, "Digest") != SkipConstantResult.FOUND) {
             return null;
         }
         // All field names are valid tokens
@@ -128,7 +128,7 @@ public class HttpParser {
             return null;
         }
         while (!field.equals("")) {
-            if (skipConstant(input, "=") != SkipResult.FOUND) {
+            if (skipConstant(input, "=") != SkipConstantResult.FOUND) {
                 return null;
             }
             String value = null;
@@ -169,7 +169,7 @@ public class HttpParser {
             }
             result.put(field, value);
 
-            if (skipConstant(input, ",") == SkipResult.NOT_FOUND) {
+            if (skipConstant(input, ",") == SkipConstantResult.NOT_FOUND) {
                 return null;
             }
             field = readToken(input);
@@ -190,7 +190,7 @@ public class HttpParser {
             return null;
         }
 
-        if (skipConstant(input, "/") == SkipResult.NOT_FOUND) {
+        if (skipConstant(input, "/") == SkipConstantResult.NOT_FOUND) {
             return null;
         }
 
@@ -203,15 +203,15 @@ public class HttpParser {
         LinkedHashMap<String,String> parameters =
                 new LinkedHashMap<String,String>();
 
-        SkipResult lookForSemiColon = skipConstant(input, ";");
-        if (lookForSemiColon == SkipResult.NOT_FOUND) {
+        SkipConstantResult lookForSemiColon = skipConstant(input, ";");
+        if (lookForSemiColon == SkipConstantResult.NOT_FOUND) {
             return null;
         }
-        while (lookForSemiColon == SkipResult.FOUND) {
+        while (lookForSemiColon == SkipConstantResult.FOUND) {
             String attribute = readToken(input);
 
             String value = "";
-            if (skipConstant(input, "=") == SkipResult.FOUND) {
+            if (skipConstant(input, "=") == SkipConstantResult.FOUND) {
                 value = readTokenOrQuotedString(input, true);
             }
 
@@ -220,7 +220,7 @@ public class HttpParser {
             }
 
             lookForSemiColon = skipConstant(input, ";");
-            if (lookForSemiColon == SkipResult.NOT_FOUND) {
+            if (lookForSemiColon == SkipConstantResult.NOT_FOUND) {
                 return null;
             }
         }
@@ -286,24 +286,25 @@ public class HttpParser {
         return c;
     }
 
-    static SkipResult skipConstant(StringReader input, String constant) throws IOException {
+    private static SkipConstantResult skipConstant(StringReader input,
+            String constant) throws IOException {
         int len = constant.length();
 
         int c = skipLws(input, false);
 
         for (int i = 0; i < len; i++) {
             if (i == 0 && c == -1) {
-                return SkipResult.EOF;
+                return SkipConstantResult.EOF;
             }
             if (c != constant.charAt(i)) {
                 input.skip(-(i + 1));
-                return SkipResult.NOT_FOUND;
+                return SkipConstantResult.NOT_FOUND;
             }
             if (i != (len - 1)) {
                 c = input.read();
             }
         }
-        return SkipResult.FOUND;
+        return SkipConstantResult.FOUND;
     }
 
     /**
@@ -311,7 +312,7 @@ public class HttpParser {
      *          available to read or <code>null</code> if data other than a
      *          token was found
      */
-    static String readToken(StringReader input) throws IOException {
+    private static String readToken(StringReader input) throws IOException {
         StringBuilder result = new StringBuilder();
 
         int c = skipLws(input, false);
@@ -371,7 +372,7 @@ public class HttpParser {
         return result.toString();
     }
 
-    static String readTokenOrQuotedString(StringReader input,
+    private static String readTokenOrQuotedString(StringReader input,
             boolean returnQuoted) throws IOException {
 
         // Go back so first non-LWS character is available to be read again
@@ -492,85 +493,7 @@ public class HttpParser {
         }
     }
 
-    static double readWeight(StringReader input, char delimiter) throws IOException {
-        int c = skipLws(input, false);
-        if (c == -1 || c == delimiter) {
-            // No q value just whitespace
-            return 1;
-        } else if (c != 'q') {
-            // Malformed. Use quality of zero so it is dropped.
-            skipUntil(input, c, delimiter);
-            return 0;
-        }
-        // RFC 7231 does not allow whitespace here but be tolerant
-        c = skipLws(input, false);
-        if (c != '=') {
-            // Malformed. Use quality of zero so it is dropped.
-            skipUntil(input, c, delimiter);
-            return 0;
-        }
-
-        // RFC 7231 does not allow whitespace here but be tolerant
-        c = skipLws(input, false);
-
-        // Should be no more than 3 decimal places
-        StringBuilder value = new StringBuilder(5);
-        int decimalPlacesRead = 0;
-        if (c == '0' || c == '1') {
-            value.append((char) c);
-            c = input.read();
-            if (c == '.') {
-                value.append('.');
-            } else if (c < '0' || c > '9') {
-                decimalPlacesRead = 3;
-            }
-            while (true) {
-                c = input.read();
-                if (c >= '0' && c <= '9') {
-                    if (decimalPlacesRead < 3) {
-                        value.append((char) c);
-                        decimalPlacesRead++;
-                    }
-                } else if (c == delimiter || c == 9 || c == 32 || c == -1) {
-                    break;
-                } else {
-                    // Go back so character is available for next read
-                    input.skip(-1);
-                    return 0;
-                }
-            }
-        } else {
-            // Malformed. Use quality of zero so it is dropped and skip until
-            // EOF or the next delimiter
-            skipUntil(input, c, delimiter);
-            return 0;
-        }
-
-        double result = Double.parseDouble(value.toString());
-        if (result > 1) {
-            return 0;
-        }
-        return result;
-    }
-
-
-    /**
-     * Skips all characters until EOF or the specified target is found. Normally
-     * used to skip invalid input until the next separator.
-     */
-    static SkipResult skipUntil(StringReader input, int c, char target) throws IOException {
-        while (c != -1 && c != target) {
-            c = input.read();
-        }
-        if (c == -1) {
-            return SkipResult.EOF;
-        } else {
-            return SkipResult.FOUND;
-        }
-    }
-
-
-    static enum SkipResult {
+    private static enum SkipConstantResult {
         FOUND,
         NOT_FOUND,
         EOF

Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/parser/MediaType.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/parser/MediaType.java?rev=1622314&r1=1622313&r2=1622314&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/parser/MediaType.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/parser/MediaType.java Wed Sep  3 18:53:56 2014
@@ -16,8 +16,6 @@
  */
 package org.apache.tomcat.util.http.parser;
 
-import java.io.IOException;
-import java.io.StringReader;
 import java.util.LinkedHashMap;
 import java.util.Locale;
 import java.util.Map;
@@ -124,56 +122,4 @@ public class MediaType {
         }
         return noCharset;
     }
-
-    /**
-     * Parses a MediaType value, either from a HTTP header or from an application.
-     *
-     * @param input a reader over the header text
-     * @return a MediaType parsed from the input, or null if not valid
-     * @throws IOException if there was a problem reading the input
-     */
-    public static MediaType parseMediaType(StringReader input) throws IOException {
-
-        // Type (required)
-        String type = HttpParser.readToken(input);
-        if (type == null || type.length() == 0) {
-            return null;
-        }
-
-        if (HttpParser.skipConstant(input, "/") == HttpParser.SkipResult.NOT_FOUND) {
-            return null;
-        }
-
-        // Subtype (required)
-        String subtype = HttpParser.readToken(input);
-        if (subtype == null || subtype.length() == 0) {
-            return null;
-        }
-
-        LinkedHashMap<String,String> parameters = new LinkedHashMap<String, String>();
-
-        HttpParser.SkipResult lookForSemiColon = HttpParser.skipConstant(input, ";");
-        if (lookForSemiColon == HttpParser.SkipResult.NOT_FOUND) {
-            return null;
-        }
-        while (lookForSemiColon == HttpParser.SkipResult.FOUND) {
-            String attribute = HttpParser.readToken(input);
-
-            String value = "";
-            if (HttpParser.skipConstant(input, "=") == HttpParser.SkipResult.FOUND) {
-                value = HttpParser.readTokenOrQuotedString(input, true);
-            }
-
-            if (attribute != null) {
-                parameters.put(attribute.toLowerCase(Locale.ENGLISH), value);
-            }
-
-            lookForSemiColon = HttpParser.skipConstant(input, ";");
-            if (lookForSemiColon == HttpParser.SkipResult.NOT_FOUND) {
-                return null;
-            }
-        }
-
-        return new MediaType(type, subtype, parameters);
-    }
 }

Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestRequest.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestRequest.java?rev=1622314&r1=1622313&r2=1622314&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestRequest.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestRequest.java Wed Sep  3 18:53:56 2014
@@ -42,7 +42,6 @@ import static org.junit.Assert.assertTru
 import static org.junit.Assert.fail;
 
 import org.junit.Assert;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import org.apache.catalina.Context;
@@ -841,24 +840,4 @@ public class TestRequest extends TomcatB
         Assert.assertEquals(expected, actual);
     }
 
-
-    @Test
-    @Ignore("Used to check performance of different parsing approaches")
-    public void localeParsePerformance() throws Exception {
-        TesterRequest req = new TesterRequest();
-        req.addHeader("accept-encoding", "en-gb,en");
-
-        long start = System.nanoTime();
-
-        // Takes about 0.3s on a quad core 2.7Ghz 2013 MacBook
-        for (int i = 0; i < 10000000; i++) {
-            req.parseLocales();
-            req.localesParsed = false;
-            req.locales.clear();
-        }
-
-        long time = System.nanoTime() - start;
-
-        System.out.println(time);
-    }
 }

Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TesterRequest.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TesterRequest.java?rev=1622314&r1=1622313&r2=1622314&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TesterRequest.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TesterRequest.java Wed Sep  3 18:53:56 2014
@@ -72,10 +72,6 @@ public class TesterRequest extends Reque
     }
     @Override
     public Enumeration<String> getHeaders(String name) {
-        List<String> values = headers.get(name);
-        if (values == null || values.size() == 0) {
-            return Collections.emptyEnumeration();
-        }
         return Collections.enumeration(headers.get(name));
     }
 

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1622314&r1=1622313&r2=1622314&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Wed Sep  3 18:53:56 2014
@@ -142,10 +142,6 @@
         Locale. (markt)
       </fix>
       <fix>
-        <bug>56848</bug>: Improve handling of <code>accept-language</code>
-        headers. (markt)
-      </fix>
-      <fix>
         Fix some potential resource leaks when reading property files. Reported
         by Coverity Scan. (violetagg)
       </fix>



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