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/08/15 14:47:56 UTC

svn commit: r1618166 - in /tomcat/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: Fri Aug 15 12:47:56 2014
New Revision: 1618166

URL: http://svn.apache.org/r1618166
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56848
Improve handling of <code>accept-language</code> headers.

Added:
    tomcat/trunk/java/org/apache/tomcat/util/http/parser/AcceptLanguage.java   (with props)
    tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestAcceptLanguage.java   (with props)
Modified:
    tomcat/trunk/java/org/apache/catalina/connector/Request.java
    tomcat/trunk/java/org/apache/tomcat/util/http/parser/Authorization.java
    tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
    tomcat/trunk/java/org/apache/tomcat/util/http/parser/MediaType.java
    tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java
    tomcat/trunk/test/org/apache/catalina/connector/TesterRequest.java
    tomcat/trunk/webapps/docs/changelog.xml

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=1618166&r1=1618165&r2=1618166&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Fri Aug 15 12:47:56 2014
@@ -21,6 +21,7 @@ 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;
@@ -76,7 +77,6 @@ import org.apache.catalina.core.Applicat
 import org.apache.catalina.core.AsyncContextImpl;
 import org.apache.catalina.mapper.MappingData;
 import org.apache.catalina.util.ParameterMap;
-import org.apache.catalina.util.StringParser;
 import org.apache.coyote.ActionCode;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
@@ -95,6 +95,7 @@ import org.apache.tomcat.util.http.fileu
 import org.apache.tomcat.util.http.fileupload.disk.DiskFileItemFactory;
 import org.apache.tomcat.util.http.fileupload.servlet.ServletFileUpload;
 import org.apache.tomcat.util.http.fileupload.servlet.ServletRequestContext;
+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;
@@ -367,12 +368,6 @@ 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;
@@ -3114,99 +3109,24 @@ public class Request
      */
     protected void parseLocalesHeader(String value, TreeMap<Double, ArrayList<Locale>> locales) {
 
-        // 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);
+        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;
         }
 
-        // 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;
-            }
-
+        for (AcceptLanguage acceptLanguage : acceptLanguages) {
             // Add a new Locale to the list of Locales for this quality level
-            Locale locale = new Locale(language, country, variant);
-            Double key = new Double(-quality);  // Reverse the order
+            Double key = new Double(-acceptLanguage.getQuality());  // Reverse the order
             ArrayList<Locale> values = locales.get(key);
             if (values == null) {
                 values = new ArrayList<>();
                 locales.put(key, values);
             }
-            values.add(locale);
+            values.add(acceptLanguage.getLocale());
         }
     }
 

Added: tomcat/trunk/java/org/apache/tomcat/util/http/parser/AcceptLanguage.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/parser/AcceptLanguage.java?rev=1618166&view=auto
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/parser/AcceptLanguage.java (added)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/parser/AcceptLanguage.java Fri Aug 15 12:47:56 2014
@@ -0,0 +1,76 @@
+/*
+ *  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.tomcat.util.http.parser;
+
+import java.io.IOException;
+import java.io.StringReader;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+
+public class AcceptLanguage {
+
+    private final Locale locale;
+    private final double quality;
+
+    protected AcceptLanguage(Locale locale, double quality) {
+        this.locale = locale;
+        this.quality = quality;
+    }
+
+    public Locale getLocale() {
+        return locale;
+    }
+
+    public double getQuality() {
+        return quality;
+    }
+
+
+    public static List<AcceptLanguage> parse(StringReader input) throws IOException {
+
+        List<AcceptLanguage> result = new ArrayList<>();
+
+        do {
+            // Token is broader than what is permitted in a language tag
+            // (alphanumeric + '-') but any invalid values that slip through
+            // will be caught later
+            String languageTag = HttpParser.readToken(input);
+            if (languageTag == null) {
+                // Invalid tag, skip to the next one
+                HttpParser.skipUntil(input, 0, ',');
+                continue;
+            }
+
+            if (languageTag.length() == 0) {
+                // No more data to read
+                break;
+            }
+
+            // See if a quality has been provided
+            double quality = 1;
+            HttpParser.SkipResult lookForSemiColon = HttpParser.skipConstant(input, ";");
+            if (lookForSemiColon == HttpParser.SkipResult.FOUND) {
+                quality = HttpParser.readWeight(input, ',');
+            }
+
+            result.add(new AcceptLanguage(Locale.forLanguageTag(languageTag), quality));
+        } while (true);
+
+        return result;
+    }
+}

Propchange: tomcat/trunk/java/org/apache/tomcat/util/http/parser/AcceptLanguage.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/java/org/apache/tomcat/util/http/parser/Authorization.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/parser/Authorization.java?rev=1618166&r1=1618165&r2=1618166&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/parser/Authorization.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/parser/Authorization.java Fri Aug 15 12:47:56 2014
@@ -78,7 +78,7 @@ public class Authorization {
 
         Map<String,String> result = new HashMap<>();
 
-        if (HttpParser.skipConstant(input, "Digest") != HttpParser.SkipConstantResult.FOUND) {
+        if (HttpParser.skipConstant(input, "Digest") != HttpParser.SkipResult.FOUND) {
             return null;
         }
         // All field names are valid tokens
@@ -87,7 +87,7 @@ public class Authorization {
             return null;
         }
         while (!field.equals("")) {
-            if (HttpParser.skipConstant(input, "=") != HttpParser.SkipConstantResult.FOUND) {
+            if (HttpParser.skipConstant(input, "=") != HttpParser.SkipResult.FOUND) {
                 return null;
             }
             String value;
@@ -127,7 +127,7 @@ public class Authorization {
             }
             result.put(field, value);
 
-            if (HttpParser.skipConstant(input, ",") == HttpParser.SkipConstantResult.NOT_FOUND) {
+            if (HttpParser.skipConstant(input, ",") == HttpParser.SkipResult.NOT_FOUND) {
                 return null;
             }
             field = HttpParser.readToken(input);

Modified: tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java?rev=1618166&r1=1618165&r2=1618166&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java Fri Aug 15 12:47:56 2014
@@ -118,24 +118,24 @@ public class HttpParser {
         return c;
     }
 
-    static SkipConstantResult skipConstant(StringReader input, String constant) throws IOException {
+    static SkipResult 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 SkipConstantResult.EOF;
+                return SkipResult.EOF;
             }
             if (c != constant.charAt(i)) {
                 input.skip(-(i + 1));
-                return SkipConstantResult.NOT_FOUND;
+                return SkipResult.NOT_FOUND;
             }
             if (i != (len - 1)) {
                 c = input.read();
             }
         }
-        return SkipConstantResult.FOUND;
+        return SkipResult.FOUND;
     }
 
     /**
@@ -321,7 +321,85 @@ public class HttpParser {
         }
     }
 
-    static enum SkipConstantResult {
+    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 {
         FOUND,
         NOT_FOUND,
         EOF

Modified: tomcat/trunk/java/org/apache/tomcat/util/http/parser/MediaType.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/parser/MediaType.java?rev=1618166&r1=1618165&r2=1618166&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/parser/MediaType.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/parser/MediaType.java Fri Aug 15 12:47:56 2014
@@ -138,7 +138,7 @@ public class MediaType {
             return null;
         }
 
-        if (HttpParser.skipConstant(input, "/") == HttpParser.SkipConstantResult.NOT_FOUND) {
+        if (HttpParser.skipConstant(input, "/") == HttpParser.SkipResult.NOT_FOUND) {
             return null;
         }
 
@@ -150,15 +150,15 @@ public class MediaType {
 
         LinkedHashMap<String,String> parameters = new LinkedHashMap<>();
 
-        HttpParser.SkipConstantResult lookForSemiColon = HttpParser.skipConstant(input, ";");
-        if (lookForSemiColon == HttpParser.SkipConstantResult.NOT_FOUND) {
+        HttpParser.SkipResult lookForSemiColon = HttpParser.skipConstant(input, ";");
+        if (lookForSemiColon == HttpParser.SkipResult.NOT_FOUND) {
             return null;
         }
-        while (lookForSemiColon == HttpParser.SkipConstantResult.FOUND) {
+        while (lookForSemiColon == HttpParser.SkipResult.FOUND) {
             String attribute = HttpParser.readToken(input);
 
             String value = "";
-            if (HttpParser.skipConstant(input, "=") == HttpParser.SkipConstantResult.FOUND) {
+            if (HttpParser.skipConstant(input, "=") == HttpParser.SkipResult.FOUND) {
                 value = HttpParser.readTokenOrQuotedString(input, true);
             }
 
@@ -167,7 +167,7 @@ public class MediaType {
             }
 
             lookForSemiColon = HttpParser.skipConstant(input, ";");
-            if (lookForSemiColon == HttpParser.SkipConstantResult.NOT_FOUND) {
+            if (lookForSemiColon == HttpParser.SkipResult.NOT_FOUND) {
                 return null;
             }
         }

Modified: tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java?rev=1618166&r1=1618165&r2=1618166&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java (original)
+++ tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java Fri Aug 15 12:47:56 2014
@@ -41,6 +41,7 @@ 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;
 import org.apache.catalina.authenticator.BasicAuthenticator;
@@ -827,4 +828,24 @@ 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/trunk/test/org/apache/catalina/connector/TesterRequest.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/connector/TesterRequest.java?rev=1618166&r1=1618165&r2=1618166&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/connector/TesterRequest.java (original)
+++ tomcat/trunk/test/org/apache/catalina/connector/TesterRequest.java Fri Aug 15 12:47:56 2014
@@ -72,6 +72,10 @@ 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));
     }
 

Added: tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestAcceptLanguage.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestAcceptLanguage.java?rev=1618166&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestAcceptLanguage.java (added)
+++ tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestAcceptLanguage.java Fri Aug 15 12:47:56 2014
@@ -0,0 +1,278 @@
+/*
+ *  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.tomcat.util.http.parser;
+
+import java.io.StringReader;
+import java.util.List;
+import java.util.Locale;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestAcceptLanguage {
+
+    private static final Locale L_EN = Locale.forLanguageTag("en");
+    private static final Locale L_EN_GB = Locale.forLanguageTag("en-gb");
+    private static final Locale L_FR = Locale.forLanguageTag("fr");
+    private static final double Q1_000 = 1;
+    private static final double Q0_500 = 0.5;
+    private static final double Q0_050 = 0.05;
+    private static final double Q0_000 = 0;
+
+    @Test
+    public void testSingle01() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testSingle02() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testSingle03() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb;"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testSingle04() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb; "));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testSingle05() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb;q=1"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testSingle06() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb; q=1"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testSingle07() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb; q= 1"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testSingle08() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb; q = 1"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testSingle09() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb; q = 1 "));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testSingle10() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb;q=0.5"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q0_500, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testSingle11() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb;q=0.50"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q0_500, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testSingle12() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb;q=0.500"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q0_500, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testSingle13() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb;q=0.5009"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q0_500, actual.get(0).getQuality(), 0.0001);
+    }
+
+
+    @Test
+    public void testMalformed01() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb;x=1"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q0_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testMalformed02() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb;q=a"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q0_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testMalformed03() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb;q=0.5a"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q0_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testMalformed04() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb;q=0.05a"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q0_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testMalformed05() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb;q=0.005a"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q0_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testMalformed06() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en-gb;q=0.00005a"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN_GB, actual.get(0).getLocale());
+        Assert.assertEquals(Q0_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testMalformed07() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en,,"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testMalformed08() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader(",en,"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testMalformed09() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader(",,en"));
+
+        Assert.assertEquals(1, actual.size());
+        Assert.assertEquals(L_EN, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+    }
+
+
+    @Test
+    public void testMultiple01() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en,fr"));
+
+        Assert.assertEquals(2, actual.size());
+        Assert.assertEquals(L_EN, actual.get(0).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(0).getQuality(), 0.0001);
+        Assert.assertEquals(L_FR, actual.get(1).getLocale());
+        Assert.assertEquals(Q1_000, actual.get(1).getQuality(), 0.0001);
+    }
+
+    @Test
+    public void testMultiple02() throws Exception {
+        List<AcceptLanguage> actual = AcceptLanguage.parse(new StringReader("en; q= 0.05,fr;q=0.5"));
+
+        Assert.assertEquals(2, actual.size());
+        Assert.assertEquals(L_EN, actual.get(0).getLocale());
+        Assert.assertEquals(Q0_050, actual.get(0).getQuality(), 0.0001);
+        Assert.assertEquals(L_FR, actual.get(1).getLocale());
+        Assert.assertEquals(Q0_500, actual.get(1).getQuality(), 0.0001);
+    }
+
+
+    @Test
+    public void bug56848() throws Exception {
+        List<AcceptLanguage> actual =
+                AcceptLanguage.parse(new StringReader("zh-hant-CN;q=0.5,zh-hans-TW;q=0.05"));
+
+        Assert.assertEquals(2, actual.size());
+
+        Locale.Builder b = new Locale.Builder();
+        b.setLanguage("zh").setRegion("CN").setScript("hant");
+        Locale l1 = b.build();
+
+        b.clear().setLanguage("zh").setRegion("TW").setScript("hans");
+        Locale l2 = b.build();
+
+        Assert.assertEquals(l1, actual.get(0).getLocale());
+        Assert.assertEquals(Q0_500, actual.get(0).getQuality(), 0.0001);
+        Assert.assertEquals(l2, actual.get(1).getLocale());
+        Assert.assertEquals(Q0_050, actual.get(1).getQuality(), 0.0001);
+    }
+}

Propchange: tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestAcceptLanguage.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1618166&r1=1618165&r2=1618166&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Aug 15 12:47:56 2014
@@ -141,6 +141,10 @@
         than just using the first header to determine the user&apos;s preferred
         Locale. (markt)
       </fix>
+      <fix>
+        <bug>56848</bug>: Improve handling of <code>accept-language</code>
+        headers. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">



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


Re: svn commit: r1618166 - in /tomcat/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/

Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-08-15 16:47 GMT+04:00  <ma...@apache.org>:
> Author: markt
> Date: Fri Aug 15 12:47:56 2014
> New Revision: 1618166
>
> URL: http://svn.apache.org/r1618166
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56848
> Improve handling of <code>accept-language</code> headers.
>
> Added:
>     tomcat/trunk/java/org/apache/tomcat/util/http/parser/AcceptLanguage.java   (with props)
>     tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestAcceptLanguage.java   (with props)
> Modified:
>     tomcat/trunk/java/org/apache/catalina/connector/Request.java
>     tomcat/trunk/java/org/apache/tomcat/util/http/parser/Authorization.java
>     tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>     tomcat/trunk/java/org/apache/tomcat/util/http/parser/MediaType.java
>     tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java
>     tomcat/trunk/test/org/apache/catalina/connector/TesterRequest.java
>     tomcat/trunk/webapps/docs/changelog.xml
>
> 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=1618166&r1=1618165&r2=1618166&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Fri Aug 15 12:47:56 2014
> @@ -21,6 +21,7 @@ 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;
> @@ -76,7 +77,6 @@ import org.apache.catalina.core.Applicat
>  import org.apache.catalina.core.AsyncContextImpl;
>  import org.apache.catalina.mapper.MappingData;
>  import org.apache.catalina.util.ParameterMap;
> -import org.apache.catalina.util.StringParser;
>  import org.apache.coyote.ActionCode;
>  import org.apache.juli.logging.Log;
>  import org.apache.juli.logging.LogFactory;
> @@ -95,6 +95,7 @@ import org.apache.tomcat.util.http.fileu
>  import org.apache.tomcat.util.http.fileupload.disk.DiskFileItemFactory;
>  import org.apache.tomcat.util.http.fileupload.servlet.ServletFileUpload;
>  import org.apache.tomcat.util.http.fileupload.servlet.ServletRequestContext;
> +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;
> @@ -367,12 +368,6 @@ public class Request
>
>
>      /**
> -     * The string parser we will use for parsing request lines.
> -     */
> -    private final StringParser parser = new StringParser();

As a result, the o.a.c.util.StringParser class is no longer used anywhere.

> -
> -
> -    /**
>       * Local port
>       */
>      protected int localPort = -1;
> @@ -3114,99 +3109,24 @@ public class Request
>       */
>      protected void parseLocalesHeader(String value, TreeMap<Double, ArrayList<Locale>> locales) {
>
> -        // 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);
> +        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;
>          }
>
> -        // 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;
> -            }
> -
> +        for (AcceptLanguage acceptLanguage : acceptLanguages) {
>              // Add a new Locale to the list of Locales for this quality level
> -            Locale locale = new Locale(language, country, variant);
> -            Double key = new Double(-quality);  // Reverse the order
> +            Double key = new Double(-acceptLanguage.getQuality());  // Reverse the order
>              ArrayList<Locale> values = locales.get(key);
>              if (values == null) {
>                  values = new ArrayList<>();
>                  locales.put(key, values);
>              }
> -            values.add(locale);
> +            values.add(acceptLanguage.getLocale());
>          }
>      }
>
>
> Added: tomcat/trunk/java/org/apache/tomcat/util/http/parser/AcceptLanguage.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/parser/AcceptLanguage.java?rev=1618166&view=auto
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/util/http/parser/AcceptLanguage.java (added)
> +++ tomcat/trunk/java/org/apache/tomcat/util/http/parser/AcceptLanguage.java Fri Aug 15 12:47:56 2014
> @@ -0,0 +1,76 @@
> +/*
> + *  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.tomcat.util.http.parser;
> +
> +import java.io.IOException;
> +import java.io.StringReader;
> +import java.util.ArrayList;
> +import java.util.List;
> +import java.util.Locale;
> +
> +public class AcceptLanguage {
> +
> +    private final Locale locale;
> +    private final double quality;
> +
> +    protected AcceptLanguage(Locale locale, double quality) {
> +        this.locale = locale;
> +        this.quality = quality;
> +    }
> +
> +    public Locale getLocale() {
> +        return locale;
> +    }
> +
> +    public double getQuality() {
> +        return quality;
> +    }
> +
> +
> +    public static List<AcceptLanguage> parse(StringReader input) throws IOException {
> +
> +        List<AcceptLanguage> result = new ArrayList<>();
> +
> +        do {
> +            // Token is broader than what is permitted in a language tag
> +            // (alphanumeric + '-') but any invalid values that slip through
> +            // will be caught later
> +            String languageTag = HttpParser.readToken(input);
> +            if (languageTag == null) {
> +                // Invalid tag, skip to the next one
> +                HttpParser.skipUntil(input, 0, ',');
> +                continue;
> +            }
> +
> +            if (languageTag.length() == 0) {
> +                // No more data to read
> +                break;
> +            }
> +
> +            // See if a quality has been provided
> +            double quality = 1;
> +            HttpParser.SkipResult lookForSemiColon = HttpParser.skipConstant(input, ";");
> +            if (lookForSemiColon == HttpParser.SkipResult.FOUND) {
> +                quality = HttpParser.readWeight(input, ',');
> +            }
> +

If readWeight() above returns 0 (invalid weight value detected),  it
would better to just skip the next line that creates an AcceptLanguage
object.

> +            result.add(new AcceptLanguage(Locale.forLanguageTag(languageTag), quality));
> +        } while (true);
> +
> +        return result;
> +    }
> +}
>

(...)

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