You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by hl...@apache.org on 2005/11/23 15:35:21 UTC

svn commit: r348451 - in /jakarta/tapestry/trunk: ./ framework/src/java/org/apache/tapestry/util/ framework/src/java/org/apache/tapestry/web/ framework/src/test/org/apache/tapestry/junit/ framework/src/test/org/apache/tapestry/web/ src/documentation/co...

Author: hlship
Date: Wed Nov 23 06:35:11 2005
New Revision: 348451

URL: http://svn.apache.org/viewcvs?rev=348451&view=rev
Log:
TAPESTRY-607: Output encoding problem with some versions of Tomcat 5

Added:
    jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/junit/ContentTypeTest.java
      - copied, changed from r332813, jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/junit/TestContentType.java
    jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/web/ServletWebResponseTest.java
      - copied, changed from r332813, jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/web/TestServletWebResponse.java
Removed:
    jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/junit/TestContentType.java
    jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/web/TestServletWebResponse.java
Modified:
    jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/util/ContentType.java
    jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/ServletWebResponse.java
    jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/WebMessages.java
    jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/WebStrings.properties
    jakarta/tapestry/trunk/src/documentation/content/xdocs/UsersGuide/configuration.xml
    jakarta/tapestry/trunk/status.xml

Modified: jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/util/ContentType.java
URL: http://svn.apache.org/viewcvs/jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/util/ContentType.java?rev=348451&r1=348450&r2=348451&view=diff
==============================================================================
--- jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/util/ContentType.java (original)
+++ jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/util/ContentType.java Wed Nov 23 06:35:11 2005
@@ -19,48 +19,63 @@
 import java.util.Set;
 import java.util.StringTokenizer;
 
+import org.apache.hivemind.util.Defense;
+
 /**
- *  Represents an HTTP content type. Allows to set various elements like
- *  the mime type, the character set, and other parameters.
- *  This is similar to a number of other implementations of the same concept in JAF, etc.
- *  We have created this simple implementation to avoid including the whole libraries. 
+ * Represents an HTTP content type. Allows to set various elements like the mime type, the character
+ * set, and other parameters. This is similar to a number of other implementations of the same
+ * concept in JAF, etc. We have created this simple implementation to avoid including the whole
+ * libraries.
  * 
- *  @author mindbridge
- *  @since 3.0
- **/
+ * @author mindbridge
+ * @since 3.0
+ */
 public class ContentType
 {
-    private String _baseType;
-    private String _subType;
-    private Map _parameters;
+    private String _baseType = "";
+
+    private String _subType = "";
+
+    private final Map _parameters = new HashMap();
 
     /**
      * Creates a new empty content type
      */
     public ContentType()
     {
-        initialize();
     }
-    
+
     /**
-     * Creates a new content type from the argument.
-     * The format of the argument has to be basetype/subtype(;key=value)* 
+     * Creates a new content type from the argument. The format of the argument has to be
+     * basetype/subtype(;key=value)*
      * 
-     * @param contentType the content type that needs to be represented
+     * @param contentType
+     *            the content type that needs to be represented
      */
     public ContentType(String contentType)
     {
         this();
         parse(contentType);
     }
-    
-    private void initialize()
+
+    /**
+     * Returns true only if the other object is another instance of ContentType, and has the ssame
+     * baseType, subType and set of parameters.
+     */
+    public boolean equals(Object o)
     {
-        _baseType = "";
-        _subType = "";
-        _parameters = new HashMap();
+        if (o == null)
+            return false;
+
+        if (o.getClass() != this.getClass())
+            return false;
+
+        ContentType ct = (ContentType) o;
+
+        return _baseType.equals(ct._baseType) && _subType.equals(ct._subType)
+                && _parameters.equals(ct._parameters);
     }
-    
+
     /**
      * @return the base type of the content type
      */
@@ -74,6 +89,8 @@
      */
     public void setBaseType(String baseType)
     {
+        Defense.notNull(baseType, "baseType");
+
         _baseType = baseType;
     }
 
@@ -90,6 +107,8 @@
      */
     public void setSubType(String subType)
     {
+        Defense.notNull(subType, "subType");
+
         _subType = subType;
     }
 
@@ -102,52 +121,64 @@
     }
 
     /**
-     * @return the list of names of parameters in this content type 
+     * @return the list of names of parameters in this content type
      */
     public String[] getParameterNames()
     {
-        Set parameterNames = _parameters.keySet(); 
+        Set parameterNames = _parameters.keySet();
         return (String[]) parameterNames.toArray(new String[parameterNames.size()]);
     }
 
     /**
-     * @param key the name of the content type parameter
+     * @param key
+     *            the name of the content type parameter
      * @return the value of the content type parameter
      */
     public String getParameter(String key)
     {
+        Defense.notNull(key, "key");
+
         return (String) _parameters.get(key);
     }
 
     /**
-     * @param key the name of the content type parameter
-     * @param value the value of the content type parameter
+     * @param key
+     *            the name of the content type parameter
+     * @param value
+     *            the value of the content type parameter
      */
     public void setParameter(String key, String value)
     {
+        Defense.notNull(key, "key");
+        Defense.notNull(value, "value");
+
         _parameters.put(key.toLowerCase(), value);
     }
 
     /**
-     * Parses the argument and configures the content type accordingly.
-     * The format of the argument has to be type/subtype(;key=value)* 
+     * Parses the argument and configures the content type accordingly. The format of the argument
+     * has to be type/subtype(;key=value)*
      * 
-     * @param contentType the content type that needs to be represented
+     * @param contentType
+     *            the content type that needs to be represented
      */
     public void parse(String contentType)
     {
-        initialize();
+        _baseType = "";
+        _subType = "";
+        _parameters.clear();
 
         StringTokenizer tokens = new StringTokenizer(contentType, ";");
-        if (!tokens.hasMoreTokens()) 
+        if (!tokens.hasMoreTokens())
             return;
-        
+
         String mimeType = tokens.nextToken();
         StringTokenizer mimeTokens = new StringTokenizer(mimeType, "/");
         setBaseType(mimeTokens.hasMoreTokens() ? mimeTokens.nextToken() : "");
         setSubType(mimeTokens.hasMoreTokens() ? mimeTokens.nextToken() : "");
-        
-        while (tokens.hasMoreTokens()) {
+
+        while (tokens.hasMoreTokens())
+        {
             String parameter = tokens.nextToken();
 
             StringTokenizer parameterTokens = new StringTokenizer(parameter, "=");
@@ -157,8 +188,6 @@
         }
     }
 
-    
-
     /**
      * @return the string representation of this content type
      */
@@ -172,11 +201,11 @@
             String key = parameterNames[i];
             String value = getParameter(key);
             buf.append(";" + key + "=" + value);
-        } 
-        
+        }
+
         return buf.toString();
     }
-    
+
     /**
      * @return the string representation of this content type. Same as unparse().
      */

Modified: jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/ServletWebResponse.java
URL: http://svn.apache.org/viewcvs/jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/ServletWebResponse.java?rev=348451&r1=348450&r2=348451&view=diff
==============================================================================
--- jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/ServletWebResponse.java (original)
+++ jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/ServletWebResponse.java Wed Nov 23 06:35:11 2005
@@ -35,17 +35,34 @@
  */
 public class ServletWebResponse implements WebResponse
 {
-    private static final Log LOG = LogFactory.getLog(ServletWebResponse.class);
+    private static final Log DEFAULT_LOG = LogFactory.getLog(ServletWebResponse.class);
+
+    private final Log _log;
+
+    private final boolean _tomcatPatch;
 
     private final HttpServletResponse _servletResponse;
 
     private boolean _needsReset;
 
+    private ContentType _printWriterContentType;
+
     public ServletWebResponse(HttpServletResponse response)
     {
+        this(response, DEFAULT_LOG, Boolean.getBoolean("org.apache.tapestry.607-patch"));
+    }
+
+    /**
+     * Alternate constructor used by some tests.
+     */
+    ServletWebResponse(HttpServletResponse response, Log log, boolean tomcatPatch)
+    {
         Defense.notNull(response, "response");
+        Defense.notNull(log, "log");
 
         _servletResponse = response;
+        _log = log;
+        _tomcatPatch = tomcatPatch;
     }
 
     public OutputStream getOutputStream(ContentType contentType)
@@ -73,8 +90,20 @@
             reset();
 
         _needsReset = true;
+        
+        if (_printWriterContentType == null || ! _tomcatPatch)
+        {
+            _servletResponse.setContentType(contentType.toString());
+            _printWriterContentType = contentType;
+        }
+        else
+        {
+            // This is a workaround for a tomcat bug; it takes effect when a page is reset so that
+            // the exception page (typically) can be rendered. See TAPESTRY-607 for details.
 
-        _servletResponse.setContentType(contentType.toString());
+            if (!_printWriterContentType.equals(contentType))
+                _log.warn(WebMessages.contentTypeUnchanged(_printWriterContentType, contentType));
+        }
 
         try
         {
@@ -100,7 +129,7 @@
         }
         catch (IllegalStateException ex)
         {
-            LOG.error(WebMessages.resetFailed(ex), ex);
+            _log.error(WebMessages.resetFailed(ex), ex);
         }
     }
 

Modified: jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/WebMessages.java
URL: http://svn.apache.org/viewcvs/jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/WebMessages.java?rev=348451&r1=348450&r2=348451&view=diff
==============================================================================
--- jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/WebMessages.java (original)
+++ jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/WebMessages.java Wed Nov 23 06:35:11 2005
@@ -59,4 +59,9 @@
     {
         return _formatter.format("reset-failed", cause);
     }
+
+    static String contentTypeUnchanged(ContentType existing, ContentType requested)
+    {
+        return _formatter.format("content-type-unchanged", existing, requested);
+    }
 }

Modified: jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/WebStrings.properties
URL: http://svn.apache.org/viewcvs/jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/WebStrings.properties?rev=348451&r1=348450&r2=348451&view=diff
==============================================================================
--- jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/WebStrings.properties (original)
+++ jakarta/tapestry/trunk/framework/src/java/org/apache/tapestry/web/WebStrings.properties Wed Nov 23 06:35:11 2005
@@ -19,3 +19,4 @@
 unable-to-find-dispatcher=Unable to find a request dispatcher for local resource ''{0}''.
 unable-to-forward=Unable to forward to local resource ''{0}'': {1}
 reset-failed=Unable to reset response buffer: {0}
+content-type-unchanged=Unable to change response content type from ''{0}'' to ''{1}'' (following a reset). See Tapestry issue TAPESTRY-607.

Copied: jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/junit/ContentTypeTest.java (from r332813, jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/junit/TestContentType.java)
URL: http://svn.apache.org/viewcvs/jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/junit/ContentTypeTest.java?p2=jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/junit/ContentTypeTest.java&p1=jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/junit/TestContentType.java&r1=332813&r2=348451&rev=348451&view=diff
==============================================================================
--- jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/junit/TestContentType.java (original)
+++ jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/junit/ContentTypeTest.java Wed Nov 23 06:35:11 2005
@@ -23,8 +23,38 @@
  * @since 3.0
  */
 
-public class TestContentType extends TapestryTestCase
+public class ContentTypeTest extends TapestryTestCase
 {
+    public void testEquals()
+    {
+        ContentType master = new ContentType("text/html");
+
+        assertEquals(false, master.equals(null));
+        assertEquals(false, master.equals(this));
+        assertEquals(true, master.equals(master));
+        assertEquals(true, master.equals(new ContentType("text/html")));
+        assertEquals(false, master.equals(new ContentType("foo/bar")));
+        assertEquals(false, master.equals(new ContentType("text/plain")));
+    }
+
+    public void testEqualsWithParameters()
+    {
+        ContentType master = new ContentType("text/html;charset=utf-8");
+
+        assertEquals(false, master.equals(new ContentType("text/html")));
+        assertEquals(true, master.equals(new ContentType("text/html;charset=utf-8")));
+        assertEquals(false, master.equals(new ContentType("text/html;charset=utf-8;foo=bar")));
+        
+        // Check that keys are case insensitive
+        
+        assertEquals(true, master.equals(new ContentType("text/html;Charset=utf-8")));
+     
+        master = new ContentType("text/html;foo=bar;biff=bazz");
+        
+        assertEquals(true, master.equals(new ContentType("text/html;foo=bar;biff=bazz")));
+        assertEquals(true, master.equals(new ContentType("text/html;Foo=bar;Biff=bazz")));
+        assertEquals(true, master.equals(new ContentType("text/html;biff=bazz;foo=bar")));
+    }
 
     public void testParsing1() throws Exception
     {

Copied: jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/web/ServletWebResponseTest.java (from r332813, jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/web/TestServletWebResponse.java)
URL: http://svn.apache.org/viewcvs/jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/web/ServletWebResponseTest.java?p2=jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/web/ServletWebResponseTest.java&p1=jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/web/TestServletWebResponse.java&r1=332813&r2=348451&rev=348451&view=diff
==============================================================================
--- jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/web/TestServletWebResponse.java (original)
+++ jakarta/tapestry/trunk/framework/src/test/org/apache/tapestry/web/ServletWebResponseTest.java Wed Nov 23 06:35:11 2005
@@ -21,6 +21,7 @@
 import javax.servlet.ServletOutputStream;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.commons.logging.Log;
 import org.apache.hivemind.ApplicationRuntimeException;
 import org.apache.hivemind.test.HiveMindTestCase;
 import org.apache.tapestry.util.ContentType;
@@ -31,7 +32,7 @@
  * @author Howard M. Lewis Ship
  * @since 4.0
  */
-public class TestServletWebResponse extends HiveMindTestCase
+public class ServletWebResponseTest extends HiveMindTestCase
 {
     private static class MockServletOutputStream extends ServletOutputStream
     {
@@ -94,8 +95,8 @@
         HttpServletResponse response = newResponse();
 
         response.setContentType("foo/bar");
-        response.getWriter();
-        setReturnValue(response, writer);
+        
+        trainGetWriter(response, writer);
 
         replayControls();
 
@@ -106,6 +107,14 @@
         verifyControls();
     }
 
+    private void trainGetWriter(HttpServletResponse response, PrintWriter writer) throws IOException
+    {
+        response.getWriter();
+        setReturnValue(response, writer);
+    }
+    
+    
+
     public void testGetSecondPrintWriter() throws Exception
     {
         PrintWriter writer1 = new PrintWriter(new CharArrayWriter());
@@ -114,9 +123,9 @@
         HttpServletResponse response = newResponse();
 
         response.setContentType("foo/bar");
-        response.getWriter();
-        setReturnValue(response, writer1);
-
+        
+        trainGetWriter(response, writer1);
+        
         replayControls();
 
         ServletWebResponse swr = new ServletWebResponse(response);
@@ -126,16 +135,85 @@
         verifyControls();
 
         response.reset();
-        response.setContentType("zip/zap");
-        response.getWriter();
-        setReturnValue(response, writer2);
+        response.setContentType("biff/bazz");
+        
+        trainGetWriter(response, writer2);
+        
+        replayControls();
+
+        assertSame(writer2, swr.getPrintWriter(new ContentType("biff/bazz")));
+
+        verifyControls();
+    }
+    
+    public void testGetSecondPrintWriterTomcatPatch() throws Exception
+    {
+        PrintWriter writer1 = new PrintWriter(new CharArrayWriter());
+        PrintWriter writer2 = new PrintWriter(new CharArrayWriter());
+
+        HttpServletResponse response = newResponse();
+        Log log = newLog();
 
+        response.setContentType("foo/bar");
+        
+        trainGetWriter(response, writer1);
+        
         replayControls();
 
-        assertSame(writer2, swr.getPrintWriter(new ContentType("zip/zap")));
+        ServletWebResponse swr = new ServletWebResponse(response, log, true);
+
+        assertSame(writer1, swr.getPrintWriter(new ContentType("foo/bar")));
+
+        verifyControls();
+
+        response.reset();
+
+        trainGetWriter(response, writer2);
+        
+        replayControls();
+
+        assertSame(writer2, swr.getPrintWriter(new ContentType("foo/bar")));
 
         verifyControls();
     }
+    
+    public void testGetSecondPrintWriterDifferentContentTypeTomcatPatch() throws Exception
+    {
+        PrintWriter writer1 = new PrintWriter(new CharArrayWriter());
+        PrintWriter writer2 = new PrintWriter(new CharArrayWriter());
+
+        HttpServletResponse response = newResponse();
+        Log log = newLog();
+
+        response.setContentType("foo/bar");
+        
+        trainGetWriter(response, writer1);
+        
+        replayControls();
+
+        ServletWebResponse swr = new ServletWebResponse(response, log, true);
+
+        assertSame(writer1, swr.getPrintWriter(new ContentType("foo/bar")));
+
+        verifyControls();
+
+        response.reset();
+        
+        log.warn("Unable to change response content type from 'foo/bar' to 'biff/bazz' (following a reset). See Tapestry issue TAPESTRY-607.");
+        
+        trainGetWriter(response, writer2);
+        
+        replayControls();
+
+        assertSame(writer2, swr.getPrintWriter(new ContentType("biff/bazz")));
+
+        verifyControls();
+    }    
+
+    private Log newLog()
+    {
+        return (Log)newMock(Log.class);
+    }    
 
     public void testGetPrintWriterFailure() throws Exception
     {

Modified: jakarta/tapestry/trunk/src/documentation/content/xdocs/UsersGuide/configuration.xml
URL: http://svn.apache.org/viewcvs/jakarta/tapestry/trunk/src/documentation/content/xdocs/UsersGuide/configuration.xml?rev=348451&r1=348450&r2=348451&view=diff
==============================================================================
--- jakarta/tapestry/trunk/src/documentation/content/xdocs/UsersGuide/configuration.xml (original)
+++ jakarta/tapestry/trunk/src/documentation/content/xdocs/UsersGuide/configuration.xml Wed Nov 23 06:35:11 2005
@@ -580,11 +580,17 @@
 	</td>
   </tr>
   
-  
-	
-
-
-
+  <tr>
+    <td>org.apache.tapestry.607-patch</td>
+    
+    <td>
+      A workaround for <link href="http://issues.apache.org/jira/browse/TAPESTRY-607">TAPESTRY-607</link>, a problem
+      related to response character sets when using some versions of Tomcat 5. The Tomcat bug is
+      <a href="http://issues.apache.org/bugzilla/show_bug.cgi?id=37072">37072</a>.  This patch ensures that
+      HttpServletResponse.setContentType() is only invoked once, even if the output is reset (for instance, to
+      switch to the Tapestry exception page).  The value must be set to true as a JVM system property.
+    </td>
+  </tr>
 
   <tr>
 	<td>org.apache.tapestry.visit-class</td>
@@ -604,18 +610,7 @@
   
   </td>
   </tr>
-
-
-
-
-
-
-
-
-
-
-
-
+  
 </table>
 </section>  <!-- configuration.properties -->
 

Modified: jakarta/tapestry/trunk/status.xml
URL: http://svn.apache.org/viewcvs/jakarta/tapestry/trunk/status.xml?rev=348451&r1=348450&r2=348451&view=diff
==============================================================================
--- jakarta/tapestry/trunk/status.xml (original)
+++ jakarta/tapestry/trunk/status.xml Wed Nov 23 06:35:11 2005
@@ -68,6 +68,7 @@
       <action type="fix" dev="HLS" fixes-bug="TAPESTRY-768">FormMessages class has typo in message key for fieldAlreadyPrerendered()</action>
       <action type="fix" dev="HLS" fixes-bug="TAPESTRY-275" due-to="Igor Grimaylo">Single quotes in a localization of DatePicker strings causes a failure</action>
       <action type="fix" dev="HLS" fixes-bug="TAPESTRY-701">NPE creating a link from pageValidate() when there are client-persistent properties with page scope</action>
+      <action type="fix" dev="HLS" fixes-bug="TAPESTRY-607">Output encoding problem with some versions of Tomcat 5</action>
     </release>
     <release version="4.0-beta-13" date="Nov 12 2005">
       <action type="update" dev="HLS">Switch to HiveMind 1.1 (final)</action>



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