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 19:37:52 UTC

svn commit: r1622302 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/connector/Request.java test/org/apache/catalina/connector/TestRequest.java test/org/apache/catalina/connector/TesterRequest.java webapps/docs/changelog.xml

Author: markt
Date: Wed Sep  3 17:37:51 2014
New Revision: 1622302

URL: http://svn.apache.org/r1622302
Log:
Correctly handle multiple accept-language headers rather than just using the first header to determine the user's preferred Locale.

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/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/
------------------------------------------------------------------------------
  Merged /tomcat/trunk:r1618112

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=1622302&r1=1622301&r2=1622302&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 17:37:51 2014
@@ -3226,26 +3226,33 @@ public class Request
 
         localesParsed = true;
 
+        // Store the accumulated languages that have been requested in
+        // a local collection, sorted by the quality value (so we can
+        // add Locales in descending order).  The values will be ArrayLists
+        // containing the corresponding Locales to be added
+        TreeMap<Double, ArrayList<Locale>> locales = new TreeMap<Double, ArrayList<Locale>>();
+
         Enumeration<String> values = getHeaders("accept-language");
 
         while (values.hasMoreElements()) {
             String value = values.nextElement();
-            parseLocalesHeader(value);
+            parseLocalesHeader(value, locales);
         }
 
+        // Process the quality values in highest->lowest order (due to
+        // negating the Double value when creating the key)
+        for (ArrayList<Locale> list : locales.values()) {
+            for (Locale locale : list) {
+                addLocale(locale);
+            }
+        }
     }
 
 
     /**
      * Parse accept-language header value.
      */
-    protected void parseLocalesHeader(String value) {
-
-        // Store the accumulated languages that have been requested in
-        // a local collection, sorted by the quality value (so we can
-        // add Locales in descending order).  The values will be ArrayLists
-        // containing the corresponding Locales to be added
-        TreeMap<Double, ArrayList<Locale>> locales = new TreeMap<Double, ArrayList<Locale>>();
+    protected void parseLocalesHeader(String value, TreeMap<Double, ArrayList<Locale>> locales) {
 
         // Preprocess the value to remove all whitespace
         int white = value.indexOf(' ');
@@ -3340,17 +3347,7 @@ public class Request
                 locales.put(key, values);
             }
             values.add(locale);
-
-        }
-
-        // Process the quality values in highest->lowest order (due to
-        // negating the Double value when creating the key)
-        for (ArrayList<Locale> list : locales.values()) {
-            for (Locale locale : list) {
-                addLocale(locale);
-            }
         }
-
     }
 
 

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=1622302&r1=1622301&r2=1622302&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 17:37:51 2014
@@ -28,6 +28,7 @@ import java.net.URL;
 import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.List;
+import java.util.Locale;
 import java.util.TreeMap;
 
 import javax.servlet.ServletException;
@@ -40,6 +41,7 @@ import static org.junit.Assert.assertNot
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import org.junit.Assert;
 import org.junit.Test;
 
 import org.apache.catalina.Context;
@@ -660,7 +662,7 @@ public class TestRequest extends TomcatB
             writer.append("Content-Disposition: form-data; name=\"part\"\r\n");
             writer.append("Content-Type: text/plain; charset=UTF-8\r\n");
             writer.append("\r\n");
-            writer.append("äö").append("\r\n");
+            writer.append("��").append("\r\n");
             writer.flush();
 
             writer.append("\r\n");
@@ -687,7 +689,7 @@ public class TestRequest extends TomcatB
                 while ((line = reader.readLine()) != null) {
                     response.add(line);
                 }
-                assertTrue(response.contains("Part äö"));
+                assertTrue(response.contains("Part ��"));
             } finally {
                 if (reader != null) {
                     reader.close();
@@ -807,4 +809,35 @@ public class TestRequest extends TomcatB
             resp.getWriter().print(req.getContextPath());
         }
     }
+
+    @Test
+    public void getLocaleMultipleHeaders01() throws Exception {
+        TesterRequest req = new TesterRequest();
+
+        req.addHeader("accept-language", "en;q=0.5");
+        req.addHeader("accept-language", "en-gb");
+
+        Locale actual = req.getLocale();
+        Locale expected = Locale.forLanguageTag("en-gb");
+
+        Assert.assertEquals(expected, actual);
+    }
+
+    /*
+     * Reverse header order of getLocaleMultipleHeaders01() and make sure the
+     * result is the same.
+     */
+    @Test
+    public void getLocaleMultipleHeaders02() throws Exception {
+        TesterRequest req = new TesterRequest();
+
+        req.addHeader("accept-language", "en-gb");
+        req.addHeader("accept-language", "en;q=0.5");
+
+        Locale actual = req.getLocale();
+        Locale expected = Locale.forLanguageTag("en-gb");
+
+        Assert.assertEquals(expected, actual);
+    }
+
 }

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=1622302&r1=1622301&r2=1622302&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 17:37:51 2014
@@ -16,6 +16,13 @@
  */
 package org.apache.catalina.connector;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 public class TesterRequest extends Request {
     @Override
     public String getScheme() {
@@ -45,4 +52,31 @@ public class TesterRequest extends Reque
     public String getMethod() {
         return method;
     }
+
+    private final Map<String,List<String>> headers = new HashMap<String, List<String>>();
+    protected void addHeader(String name, String value) {
+        List<String> values = headers.get(name);
+        if (values == null) {
+            values = new ArrayList<String>();
+            headers.put(name, values);
+        }
+        values.add(value);
+    }
+    @Override
+    public String getHeader(String name) {
+        List<String> values = headers.get(name);
+        if (values == null || values.size() == 0) {
+            return null;
+        }
+        return values.get(0);
+    }
+    @Override
+    public Enumeration<String> getHeaders(String name) {
+        return Collections.enumeration(headers.get(name));
+    }
+
+    @Override
+    public Enumeration<String> getHeaderNames() {
+        return Collections.enumeration(headers.keySet());
+    }
 }

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=1622302&r1=1622301&r2=1622302&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Wed Sep  3 17:37:51 2014
@@ -136,6 +136,11 @@
         Fix a potential resource leak when reading MANIFEST.MF file for
         extension dependencies reported by Coverity Scan. (violetagg)
       </fix>
+      <fix>
+        Correctly handle multiple <code>accept-language</code> headers rather
+        than just using the first header to determine the user&apos;s preferred
+        Locale. (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: r1622302 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/connector/Request.java test/org/apache/catalina/connector/TestRequest.java test/org/apache/catalina/connector/TesterRequest.java webapps/docs/changelog.xml

Posted by sebb <se...@gmail.com>.
On 4 September 2014 10:52, Mark Thomas <ma...@apache.org> wrote:
> On 04/09/2014 10:50, Mark Thomas wrote:
>> On 04/09/2014 10:16, Mark Thomas wrote:
>>> On 04/09/2014 10:11, Mark Thomas wrote:
>>>> On 04/09/2014 08:05, Martin Grigorov wrote:
>>>>> On Wed, Sep 3, 2014 at 8:37 PM, <ma...@apache.org> wrote:
>>>>>
>>>>>> Author: markt
>>>>>> Date: Wed Sep  3 17:37:51 2014
>>>>>> New Revision: 1622302
>>>>
>>>> <snip/>
>>>>>> ---
>>>>>> 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 17:37:51 2014
>>>>>> @@ -28,6 +28,7 @@ import java.net.URL;
>>>>>>  import java.util.ArrayList;
>>>>>>  import java.util.Enumeration;
>>>>>>  import java.util.List;
>>>>>> +import java.util.Locale;
>>>>>>  import java.util.TreeMap;
>>>>>>
>>>>>>  import javax.servlet.ServletException;
>>>>>> @@ -40,6 +41,7 @@ import static org.junit.Assert.assertNot
>>>>>>  import static org.junit.Assert.assertTrue;
>>>>>>  import static org.junit.Assert.fail;
>>>>>>
>>>>>> +import org.junit.Assert;
>>>>>>  import org.junit.Test;
>>>>>>
>>>>>>  import org.apache.catalina.Context;
>>>>>> @@ -660,7 +662,7 @@ public class TestRequest extends TomcatB
>>>>>>              writer.append("Content-Disposition: form-data;
>>>>>> name=\"part\"\r\n");
>>>>>>              writer.append("Content-Type: text/plain; charset=UTF-8\r\n");
>>>>>>              writer.append("\r\n");
>>>>>> -            writer.append("äö").append("\r\n");
>>>>>> +            writer.append("��").append("\r\n");
>>>>>>
>>>>>
>>>>> It looks like there is an encoding issue here ?!
>>>>
>>>> No. There is a known issue with the code that generates the commit mails
>>>> and UTF-8.
>>>
>>> Saying that, that diff does look a little odd. The original looks more
>>> reasonable but looking back at the history with viewvc shows the same
>>> unprintable characters throughout which is odd.
>>>
>>> I wonder if there is a platform related thing going on here. The tests
>>> pass - which is the main thing - but I'll do some more digging on this.
>>
>> Looks like the commit mailer issue is fixed (I haven't checked for a
>> while).
>
> I take that back. The mailer isn't fixed. viewvc is fine.

The SVN mailer needs to know the file encoding.

For a specific file, one can add a property:

svn pset svnmailer:content-charset utf-8  TestRequest.java

If this is not defined, then the mailer looks for the property in
parent directories [1]

It's possible that the default it is detecting is ASCII or similar.

[1] http://opensource.perlig.de/svnmailer/doc-1.0/#groups-charset-property

> Mark
>
>> viewvc also shows UTF-8 correctly as well.
>>
>> Moral of this thread. If we see something that looks like character
>> corruption in commit logs or viewvc then now it probably is.
>>
>> Note that Windows uses cp1252 by default rather than UTF-8. I suspect
>> that that is where the corruption sneaked in and I have changed the
>> default for my Windows dev environment.
>>
>> Corruption fixed in trunk and 7.0.x.
>>
>> Mark
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>

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


Re: svn commit: r1622302 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/connector/Request.java test/org/apache/catalina/connector/TestRequest.java test/org/apache/catalina/connector/TesterRequest.java webapps/docs/changelog.xml

Posted by Mark Thomas <ma...@apache.org>.
On 04/09/2014 10:50, Mark Thomas wrote:
> On 04/09/2014 10:16, Mark Thomas wrote:
>> On 04/09/2014 10:11, Mark Thomas wrote:
>>> On 04/09/2014 08:05, Martin Grigorov wrote:
>>>> On Wed, Sep 3, 2014 at 8:37 PM, <ma...@apache.org> wrote:
>>>>
>>>>> Author: markt
>>>>> Date: Wed Sep  3 17:37:51 2014
>>>>> New Revision: 1622302
>>>
>>> <snip/>
>>>>> ---
>>>>> 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 17:37:51 2014
>>>>> @@ -28,6 +28,7 @@ import java.net.URL;
>>>>>  import java.util.ArrayList;
>>>>>  import java.util.Enumeration;
>>>>>  import java.util.List;
>>>>> +import java.util.Locale;
>>>>>  import java.util.TreeMap;
>>>>>
>>>>>  import javax.servlet.ServletException;
>>>>> @@ -40,6 +41,7 @@ import static org.junit.Assert.assertNot
>>>>>  import static org.junit.Assert.assertTrue;
>>>>>  import static org.junit.Assert.fail;
>>>>>
>>>>> +import org.junit.Assert;
>>>>>  import org.junit.Test;
>>>>>
>>>>>  import org.apache.catalina.Context;
>>>>> @@ -660,7 +662,7 @@ public class TestRequest extends TomcatB
>>>>>              writer.append("Content-Disposition: form-data;
>>>>> name=\"part\"\r\n");
>>>>>              writer.append("Content-Type: text/plain; charset=UTF-8\r\n");
>>>>>              writer.append("\r\n");
>>>>> -            writer.append("äö").append("\r\n");
>>>>> +            writer.append("��").append("\r\n");
>>>>>
>>>>
>>>> It looks like there is an encoding issue here ?!
>>>
>>> No. There is a known issue with the code that generates the commit mails
>>> and UTF-8.
>>
>> Saying that, that diff does look a little odd. The original looks more
>> reasonable but looking back at the history with viewvc shows the same
>> unprintable characters throughout which is odd.
>>
>> I wonder if there is a platform related thing going on here. The tests
>> pass - which is the main thing - but I'll do some more digging on this.
> 
> Looks like the commit mailer issue is fixed (I haven't checked for a
> while).

I take that back. The mailer isn't fixed. viewvc is fine.

Mark

> viewvc also shows UTF-8 correctly as well.
> 
> Moral of this thread. If we see something that looks like character
> corruption in commit logs or viewvc then now it probably is.
> 
> Note that Windows uses cp1252 by default rather than UTF-8. I suspect
> that that is where the corruption sneaked in and I have changed the
> default for my Windows dev environment.
> 
> Corruption fixed in trunk and 7.0.x.
> 
> Mark
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 


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


Re: svn commit: r1622302 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/connector/Request.java test/org/apache/catalina/connector/TestRequest.java test/org/apache/catalina/connector/TesterRequest.java webapps/docs/changelog.xml

Posted by Mark Thomas <ma...@apache.org>.
On 04/09/2014 10:16, Mark Thomas wrote:
> On 04/09/2014 10:11, Mark Thomas wrote:
>> On 04/09/2014 08:05, Martin Grigorov wrote:
>>> On Wed, Sep 3, 2014 at 8:37 PM, <ma...@apache.org> wrote:
>>>
>>>> Author: markt
>>>> Date: Wed Sep  3 17:37:51 2014
>>>> New Revision: 1622302
>>
>> <snip/>
>>>> ---
>>>> 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 17:37:51 2014
>>>> @@ -28,6 +28,7 @@ import java.net.URL;
>>>>  import java.util.ArrayList;
>>>>  import java.util.Enumeration;
>>>>  import java.util.List;
>>>> +import java.util.Locale;
>>>>  import java.util.TreeMap;
>>>>
>>>>  import javax.servlet.ServletException;
>>>> @@ -40,6 +41,7 @@ import static org.junit.Assert.assertNot
>>>>  import static org.junit.Assert.assertTrue;
>>>>  import static org.junit.Assert.fail;
>>>>
>>>> +import org.junit.Assert;
>>>>  import org.junit.Test;
>>>>
>>>>  import org.apache.catalina.Context;
>>>> @@ -660,7 +662,7 @@ public class TestRequest extends TomcatB
>>>>              writer.append("Content-Disposition: form-data;
>>>> name=\"part\"\r\n");
>>>>              writer.append("Content-Type: text/plain; charset=UTF-8\r\n");
>>>>              writer.append("\r\n");
>>>> -            writer.append("äö").append("\r\n");
>>>> +            writer.append("��").append("\r\n");
>>>>
>>>
>>> It looks like there is an encoding issue here ?!
>>
>> No. There is a known issue with the code that generates the commit mails
>> and UTF-8.
> 
> Saying that, that diff does look a little odd. The original looks more
> reasonable but looking back at the history with viewvc shows the same
> unprintable characters throughout which is odd.
> 
> I wonder if there is a platform related thing going on here. The tests
> pass - which is the main thing - but I'll do some more digging on this.

Looks like the commit mailer issue is fixed (I haven't checked for a
while). viewvc also shows UTF-8 correctly as well.

Moral of this thread. If we see something that looks like character
corruption in commit logs or viewvc then now it probably is.

Note that Windows uses cp1252 by default rather than UTF-8. I suspect
that that is where the corruption sneaked in and I have changed the
default for my Windows dev environment.

Corruption fixed in trunk and 7.0.x.

Mark


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


Re: svn commit: r1622302 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/connector/Request.java test/org/apache/catalina/connector/TestRequest.java test/org/apache/catalina/connector/TesterRequest.java webapps/docs/changelog.xml

Posted by Mark Thomas <ma...@apache.org>.
On 04/09/2014 10:11, Mark Thomas wrote:
> On 04/09/2014 08:05, Martin Grigorov wrote:
>> On Wed, Sep 3, 2014 at 8:37 PM, <ma...@apache.org> wrote:
>>
>>> Author: markt
>>> Date: Wed Sep  3 17:37:51 2014
>>> New Revision: 1622302
> 
> <snip/>
>>> ---
>>> 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 17:37:51 2014
>>> @@ -28,6 +28,7 @@ import java.net.URL;
>>>  import java.util.ArrayList;
>>>  import java.util.Enumeration;
>>>  import java.util.List;
>>> +import java.util.Locale;
>>>  import java.util.TreeMap;
>>>
>>>  import javax.servlet.ServletException;
>>> @@ -40,6 +41,7 @@ import static org.junit.Assert.assertNot
>>>  import static org.junit.Assert.assertTrue;
>>>  import static org.junit.Assert.fail;
>>>
>>> +import org.junit.Assert;
>>>  import org.junit.Test;
>>>
>>>  import org.apache.catalina.Context;
>>> @@ -660,7 +662,7 @@ public class TestRequest extends TomcatB
>>>              writer.append("Content-Disposition: form-data;
>>> name=\"part\"\r\n");
>>>              writer.append("Content-Type: text/plain; charset=UTF-8\r\n");
>>>              writer.append("\r\n");
>>> -            writer.append("äö").append("\r\n");
>>> +            writer.append("��").append("\r\n");
>>>
>>
>> It looks like there is an encoding issue here ?!
> 
> No. There is a known issue with the code that generates the commit mails
> and UTF-8.

Saying that, that diff does look a little odd. The original looks more
reasonable but looking back at the history with viewvc shows the same
unprintable characters throughout which is odd.

I wonder if there is a platform related thing going on here. The tests
pass - which is the main thing - but I'll do some more digging on this.

Mark


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


Re: svn commit: r1622302 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/connector/Request.java test/org/apache/catalina/connector/TestRequest.java test/org/apache/catalina/connector/TesterRequest.java webapps/docs/changelog.xml

Posted by Mark Thomas <ma...@apache.org>.
On 04/09/2014 08:05, Martin Grigorov wrote:
> On Wed, Sep 3, 2014 at 8:37 PM, <ma...@apache.org> wrote:
> 
>> Author: markt
>> Date: Wed Sep  3 17:37:51 2014
>> New Revision: 1622302

<snip/>
>> ---
>> 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 17:37:51 2014
>> @@ -28,6 +28,7 @@ import java.net.URL;
>>  import java.util.ArrayList;
>>  import java.util.Enumeration;
>>  import java.util.List;
>> +import java.util.Locale;
>>  import java.util.TreeMap;
>>
>>  import javax.servlet.ServletException;
>> @@ -40,6 +41,7 @@ import static org.junit.Assert.assertNot
>>  import static org.junit.Assert.assertTrue;
>>  import static org.junit.Assert.fail;
>>
>> +import org.junit.Assert;
>>  import org.junit.Test;
>>
>>  import org.apache.catalina.Context;
>> @@ -660,7 +662,7 @@ public class TestRequest extends TomcatB
>>              writer.append("Content-Disposition: form-data;
>> name=\"part\"\r\n");
>>              writer.append("Content-Type: text/plain; charset=UTF-8\r\n");
>>              writer.append("\r\n");
>> -            writer.append("äö").append("\r\n");
>> +            writer.append("��").append("\r\n");
>>
> 
> It looks like there is an encoding issue here ?!

No. There is a known issue with the code that generates the commit mails
and UTF-8.

Mark

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


Re: svn commit: r1622302 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/connector/Request.java test/org/apache/catalina/connector/TestRequest.java test/org/apache/catalina/connector/TesterRequest.java webapps/docs/changelog.xml

Posted by Martin Grigorov <mg...@apache.org>.
Hi,

On Wed, Sep 3, 2014 at 8:37 PM, <ma...@apache.org> wrote:

> Author: markt
> Date: Wed Sep  3 17:37:51 2014
> New Revision: 1622302
>
> URL: http://svn.apache.org/r1622302
> Log:
> Correctly handle multiple accept-language headers rather than just using
> the first header to determine the user's preferred Locale.
>
> 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/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/
>
> ------------------------------------------------------------------------------
>   Merged /tomcat/trunk:r1618112
>
> 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=1622302&r1=1622301&r2=1622302&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 17:37:51 2014
> @@ -3226,26 +3226,33 @@ public class Request
>
>          localesParsed = true;
>
> +        // Store the accumulated languages that have been requested in
> +        // a local collection, sorted by the quality value (so we can
> +        // add Locales in descending order).  The values will be
> ArrayLists
> +        // containing the corresponding Locales to be added
> +        TreeMap<Double, ArrayList<Locale>> locales = new TreeMap<Double,
> ArrayList<Locale>>();
> +
>          Enumeration<String> values = getHeaders("accept-language");
>
>          while (values.hasMoreElements()) {
>              String value = values.nextElement();
> -            parseLocalesHeader(value);
> +            parseLocalesHeader(value, locales);
>          }
>
> +        // Process the quality values in highest->lowest order (due to
> +        // negating the Double value when creating the key)
> +        for (ArrayList<Locale> list : locales.values()) {
> +            for (Locale locale : list) {
> +                addLocale(locale);
> +            }
> +        }
>      }
>
>
>      /**
>       * Parse accept-language header value.
>       */
> -    protected void parseLocalesHeader(String value) {
> -
> -        // Store the accumulated languages that have been requested in
> -        // a local collection, sorted by the quality value (so we can
> -        // add Locales in descending order).  The values will be
> ArrayLists
> -        // containing the corresponding Locales to be added
> -        TreeMap<Double, ArrayList<Locale>> locales = new TreeMap<Double,
> ArrayList<Locale>>();
> +    protected void parseLocalesHeader(String value, TreeMap<Double,
> ArrayList<Locale>> locales) {
>
>          // Preprocess the value to remove all whitespace
>          int white = value.indexOf(' ');
> @@ -3340,17 +3347,7 @@ public class Request
>                  locales.put(key, values);
>              }
>              values.add(locale);
> -
> -        }
> -
> -        // Process the quality values in highest->lowest order (due to
> -        // negating the Double value when creating the key)
> -        for (ArrayList<Locale> list : locales.values()) {
> -            for (Locale locale : list) {
> -                addLocale(locale);
> -            }
>          }
> -
>      }
>
>
>
> 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=1622302&r1=1622301&r2=1622302&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 17:37:51 2014
> @@ -28,6 +28,7 @@ import java.net.URL;
>  import java.util.ArrayList;
>  import java.util.Enumeration;
>  import java.util.List;
> +import java.util.Locale;
>  import java.util.TreeMap;
>
>  import javax.servlet.ServletException;
> @@ -40,6 +41,7 @@ import static org.junit.Assert.assertNot
>  import static org.junit.Assert.assertTrue;
>  import static org.junit.Assert.fail;
>
> +import org.junit.Assert;
>  import org.junit.Test;
>
>  import org.apache.catalina.Context;
> @@ -660,7 +662,7 @@ public class TestRequest extends TomcatB
>              writer.append("Content-Disposition: form-data;
> name=\"part\"\r\n");
>              writer.append("Content-Type: text/plain; charset=UTF-8\r\n");
>              writer.append("\r\n");
> -            writer.append("äö").append("\r\n");
> +            writer.append("��").append("\r\n");
>

It looks like there is an encoding issue here ?!


>              writer.flush();
>
>              writer.append("\r\n");
> @@ -687,7 +689,7 @@ public class TestRequest extends TomcatB
>                  while ((line = reader.readLine()) != null) {
>                      response.add(line);
>                  }
> -                assertTrue(response.contains("Part äö"));
> +                assertTrue(response.contains("Part ��"));
>

same here


>              } finally {
>                  if (reader != null) {
>                      reader.close();
> @@ -807,4 +809,35 @@ public class TestRequest extends TomcatB
>              resp.getWriter().print(req.getContextPath());
>          }
>      }
> +
> +    @Test
> +    public void getLocaleMultipleHeaders01() throws Exception {
> +        TesterRequest req = new TesterRequest();
> +
> +        req.addHeader("accept-language", "en;q=0.5");
> +        req.addHeader("accept-language", "en-gb");
> +
> +        Locale actual = req.getLocale();
> +        Locale expected = Locale.forLanguageTag("en-gb");
> +
> +        Assert.assertEquals(expected, actual);
> +    }
> +
> +    /*
> +     * Reverse header order of getLocaleMultipleHeaders01() and make sure
> the
> +     * result is the same.
> +     */
> +    @Test
> +    public void getLocaleMultipleHeaders02() throws Exception {
> +        TesterRequest req = new TesterRequest();
> +
> +        req.addHeader("accept-language", "en-gb");
> +        req.addHeader("accept-language", "en;q=0.5");
> +
> +        Locale actual = req.getLocale();
> +        Locale expected = Locale.forLanguageTag("en-gb");
> +
> +        Assert.assertEquals(expected, actual);
> +    }
> +
>  }
>
> 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=1622302&r1=1622301&r2=1622302&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 17:37:51 2014
> @@ -16,6 +16,13 @@
>   */
>  package org.apache.catalina.connector;
>
> +import java.util.ArrayList;
> +import java.util.Collections;
> +import java.util.Enumeration;
> +import java.util.HashMap;
> +import java.util.List;
> +import java.util.Map;
> +
>  public class TesterRequest extends Request {
>      @Override
>      public String getScheme() {
> @@ -45,4 +52,31 @@ public class TesterRequest extends Reque
>      public String getMethod() {
>          return method;
>      }
> +
> +    private final Map<String,List<String>> headers = new HashMap<String,
> List<String>>();
> +    protected void addHeader(String name, String value) {
> +        List<String> values = headers.get(name);
> +        if (values == null) {
> +            values = new ArrayList<String>();
> +            headers.put(name, values);
> +        }
> +        values.add(value);
> +    }
> +    @Override
> +    public String getHeader(String name) {
> +        List<String> values = headers.get(name);
> +        if (values == null || values.size() == 0) {
> +            return null;
> +        }
> +        return values.get(0);
> +    }
> +    @Override
> +    public Enumeration<String> getHeaders(String name) {
> +        return Collections.enumeration(headers.get(name));
> +    }
> +
> +    @Override
> +    public Enumeration<String> getHeaderNames() {
> +        return Collections.enumeration(headers.keySet());
> +    }
>  }
>
> 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=1622302&r1=1622301&r2=1622302&view=diff
>
> ==============================================================================
> --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Wed Sep  3 17:37:51
> 2014
> @@ -136,6 +136,11 @@
>          Fix a potential resource leak when reading MANIFEST.MF file for
>          extension dependencies reported by Coverity Scan. (violetagg)
>        </fix>
> +      <fix>
> +        Correctly handle multiple <code>accept-language</code> headers
> rather
> +        than just using the first header to determine the user&apos;s
> preferred
> +        Locale. (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
>
>