You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by sc...@apache.org on 2010/12/03 17:07:51 UTC
svn commit: r1041892 - in /tomcat/trunk: java/org/apache/catalina/connector/
test/org/apache/catalina/connector/ webapps/docs/ webapps/docs/config/
Author: schultz
Date: Fri Dec 3 16:07:50 2010
New Revision: 1041892
URL: http://svn.apache.org/viewvc?rev=1041892&view=rev
Log:
Fixed bug 48692: Provide option to parse application/x-www-form-urlencoded PUT requests
Modified:
tomcat/trunk/java/org/apache/catalina/connector/Connector.java
tomcat/trunk/java/org/apache/catalina/connector/Request.java
tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java
tomcat/trunk/webapps/docs/changelog.xml
tomcat/trunk/webapps/docs/config/ajp.xml
tomcat/trunk/webapps/docs/config/http.xml
Modified: tomcat/trunk/java/org/apache/catalina/connector/Connector.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Connector.java?rev=1041892&r1=1041891&r2=1041892&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Connector.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Connector.java Fri Dec 3 16:07:50 2010
@@ -18,7 +18,9 @@
package org.apache.catalina.connector;
+import java.util.Arrays;
import java.util.HashMap;
+import java.util.HashSet;
import javax.management.ObjectName;
@@ -38,7 +40,7 @@ import org.apache.tomcat.util.res.String
/**
- * Implementation of a Coyote connector for Tomcat 5.x.
+ * Implementation of a Coyote connector.
*
* @author Craig R. McClanahan
* @author Remy Maucherat
@@ -184,6 +186,11 @@ public class Connector extends Lifecycle
protected int maxSavePostSize = 4 * 1024;
+ protected String parseBodyMethods = "POST";
+
+ protected HashSet parseBodyMethodsSet;
+
+
/**
* Flag to use IP-based virtual hosting.
*/
@@ -449,6 +456,30 @@ public class Connector extends Lifecycle
}
+ public String getParseBodyMethods()
+ {
+ return (this.parseBodyMethods);
+ }
+
+ public void setParseBodyMethods(String methods)
+ {
+ HashSet methodSet = new HashSet();
+
+ if(null != methods)
+ methodSet.addAll(Arrays.asList(methods.split("\\s*,\\s*")));
+
+ if(methodSet.contains("TRACE"))
+ throw new IllegalArgumentException("TRACE method MUST NOT include an entity (see RFC 2616 Section 9.6)");
+
+ this.parseBodyMethods = methods;
+ this.parseBodyMethodsSet = methodSet;
+ }
+
+ public boolean isParseBodyMethod(String method)
+ {
+ return parseBodyMethodsSet.contains(method);
+ }
+
/**
* Return the port number on which we listen for requests.
*/
@@ -866,6 +897,10 @@ public class Connector extends Lifecycle
protocolHandler.setAdapter(adapter);
protocolHandler.setDomain(getDomain());
+ // Make sure parseBodyMethodsSet has a default
+ if(null == parseBodyMethodsSet)
+ setParseBodyMethods(getParseBodyMethods());
+
try {
protocolHandler.init();
} catch (Exception e) {
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=1041892&r1=1041891&r2=1041892&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Fri Dec 3 16:07:50 2010
@@ -2798,7 +2798,7 @@ public class Request
if (usingInputStream || usingReader)
return;
- if (!getMethod().equalsIgnoreCase("POST"))
+ if(!getConnector().isParseBodyMethod(getMethod()))
return;
String contentType = getContentType();
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=1041892&r1=1041891&r2=1041892&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java (original)
+++ tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java Fri Dec 3 16:07:50 2010
@@ -23,6 +23,7 @@ import java.io.PrintWriter;
import java.net.HttpURLConnection;
import java.net.URL;
import java.util.Enumeration;
+import java.util.TreeMap;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
@@ -331,6 +332,198 @@ public class TestRequest extends TomcatB
assertNotNull(is);
}
+ /**
+ * Test case for https://issues.apache.org/bugzilla/show_bug.cgi?id=48692
+ * PUT requests should be able to fetch request parameters coming from
+ * the request body (when properly configured using the new parseBodyMethod
+ * setting).
+ */
+ public void testBug48692() {
+ Bug48692Client client = new Bug48692Client();
+ client.setPort(getPort());
+
+ // Make sure GET works properly
+ client.doRequest("GET", "foo=bar", null, null, false);
+
+ assertTrue("Non-200 response for GET request",
+ client.isResponse200());
+ assertEquals("Incorrect response for GET request",
+ "foo=bar",
+ client.getResponseBody());
+
+ client.reset();
+
+ //
+ // Make sure POST works properly
+ //
+ // POST with separate GET and POST parameters
+ client.doRequest("POST", "foo=bar", "application/x-www-form-urlencoded", "bar=baz", true);
+
+ assertTrue("Non-200 response for POST request",
+ client.isResponse200());
+ assertEquals("Incorrect response for POST request",
+ "bar=baz,foo=bar",
+ client.getResponseBody());
+
+ client.reset();
+
+ // POST with overlapping GET and POST parameters
+ client.doRequest("POST", "foo=bar&bar=foo", "application/x-www-form-urlencoded", "bar=baz&foo=baz", true);
+
+ assertTrue("Non-200 response for POST request",
+ client.isResponse200());
+ assertEquals("Incorrect response for POST request",
+ "bar=baz,bar=foo,foo=bar,foo=baz",
+ client.getResponseBody());
+
+ client.reset();
+
+ // PUT without POST-style parsing
+ client.doRequest("PUT", "foo=bar&bar=foo", "application/x-www-form-urlencoded", "bar=baz&foo=baz", false);
+
+ assertTrue("Non-200 response for PUT/noparse request",
+ client.isResponse200());
+ assertEquals("Incorrect response for PUT request",
+ "bar=foo,foo=bar",
+ client.getResponseBody());
+
+ client.reset();
+
+ // PUT with POST-style parsing
+ client.doRequest("PUT", "foo=bar&bar=foo", "application/x-www-form-urlencoded", "bar=baz&foo=baz", true);
+
+ assertTrue("Non-200 response for PUT request",
+ client.isResponse200());
+ assertEquals("Incorrect response for PUT/parse request",
+ "bar=baz,bar=foo,foo=bar,foo=baz",
+ client.getResponseBody());
+
+ client.reset();
+
+ /*
+ private Exception doRequest(String method,
+ String queryString,
+ String contentType,
+ String requestBody,
+ boolean allowBody) {
+ */
+ }
+
+ /**
+ *
+ */
+ private static class EchoParametersServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ /**
+ * Only interested in the parameters and values for requests.
+ * Note: echos parameters in alphabetical order.
+ */
+ @Override
+ protected void service(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+ // Just echo the parameters and values back as plain text
+ resp.setContentType("text/plain");
+ resp.setCharacterEncoding("UTF-8");
+
+ PrintWriter out = resp.getWriter();
+
+ TreeMap<String,String[]> parameters = new TreeMap<String,String[]>(req.getParameterMap());
+
+ boolean first = true;
+
+ for(String name: parameters.keySet()) {
+ String[] values = req.getParameterValues(name);
+
+ java.util.Arrays.sort(values);
+
+ for(int i=0; i<values.length; ++i)
+ {
+ if(first)
+ first = false;
+ else
+ out.print(",");
+
+ out.print(name + "=" + values[i]);
+ }
+ }
+ }
+ }
+
+ /**
+ * Bug 48692 test client: test for allowing PUT request bodies.
+ */
+ private class Bug48692Client extends SimpleHttpClient {
+
+ private boolean init;
+
+ private synchronized void init() throws Exception {
+ if (init) return;
+
+ Tomcat tomcat = getTomcatInstance();
+ Context root = tomcat.addContext("", TEMP_DIR);
+ Tomcat.addServlet(root, "EchoParameters", new EchoParametersServlet());
+ root.addServletMapping("/echo", "EchoParameters");
+ tomcat.start();
+
+ init = true;
+ }
+
+ private Exception doRequest(String method,
+ String queryString,
+ String contentType,
+ String requestBody,
+ boolean allowBody) {
+ Tomcat tomcat = getTomcatInstance();
+
+ try {
+ init();
+ if(allowBody)
+ tomcat.getConnector().setParseBodyMethods(method);
+ else
+ tomcat.getConnector().setParseBodyMethods(""); // never parse
+
+ // Open connection
+ connect();
+
+ // Re-encode the request body so that bytes = characters
+ if(null != requestBody)
+ requestBody = new String(requestBody.getBytes("UTF-8"), "ASCII");
+
+ // Send specified request body using method
+ String[] request = {
+ (
+ method + " http://localhost:" + getPort() + "/echo"
+ + (null == queryString ? "" : ("?" + queryString))
+ + " HTTP/1.1" + CRLF
+ + "Host: localhost" + CRLF
+ + (null == contentType ? ""
+ : ("Content-Type: " + contentType + CRLF))
+ + "Connection: close" + CRLF
+ + (null == requestBody ? "" : "Content-Length: " + requestBody.length() + CRLF)
+ + CRLF
+ + (null == requestBody ? "" : requestBody)
+ )
+ };
+
+ setRequest(request);
+ processRequest(); // blocks until response has been read
+
+ // Close the connection
+ disconnect();
+ } catch (Exception e) {
+ return e;
+ }
+ return null;
+ }
+
+ @Override
+ public boolean isResponseBodyOK() {
+ return false; // Don't care
+ }
+ }
+
private HttpURLConnection getConnection() throws IOException {
final String query = "http://localhost:" + getPort() + "/";
URL postURL;
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1041892&r1=1041891&r2=1041892&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Dec 3 16:07:50 2010
@@ -32,6 +32,7 @@
<author email="kfujino@apache.org">Keiichi Fujino</author>
<author email="timw@apache.org">Tim Whittington</author>
<author email="mturk@apache.org">Mladen Turk</author>
+ <author email="schultz@apache.org">Christopher Schultz</author>
<title>Changelog</title>
</properties>
@@ -40,6 +41,10 @@
<section name="Tomcat 7.0.6 (markt)">
<subsection name="Catalina">
<changelog>
+ <update>
+ <bug>48692</bug>: Provide option to parse
+ <code>application/x-www-form-urlencoded</code> PUT requests. (schultz)
+ </update>
<fix>
<bug>8705</bug>: <code>org.apache.catalina.SessionListener</code> now
extends <code>java.util.EventListener</code>. (markt)
Modified: tomcat/trunk/webapps/docs/config/ajp.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/ajp.xml?rev=1041892&r1=1041891&r2=1041892&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/ajp.xml (original)
+++ tomcat/trunk/webapps/docs/config/ajp.xml Fri Dec 3 16:07:50 2010
@@ -115,6 +115,18 @@
to 4096 (4 kilobytes).</p>
</attribute>
+ <attribute name="parseBodyMethods" required="false">
+ <p>A comma-separated list of HTTP methods for which request
+ bodies will be parsed for request parameters identically
+ to POST. This is useful in RESTful applications that want to
+ support POST-style semantics for PUT requests.
+ Note that any setting other than <code>POST</code> causes Tomcat
+ to behave in a way that violates the servlet specification.
+ The HTTP method TRACE is specifically forbidden here in accordance
+ with the HTTP specification.
+ The default is <code>POST</code></p>
+ </attribute>
+
<attribute name="port" required="true">
<p>The TCP port number on which this <strong>Connector</strong>
will create a server socket and await incoming connections. Your
Modified: tomcat/trunk/webapps/docs/config/http.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/http.xml?rev=1041892&r1=1041891&r2=1041892&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/http.xml (original)
+++ tomcat/trunk/webapps/docs/config/http.xml Fri Dec 3 16:07:50 2010
@@ -115,6 +115,18 @@
to 4096 (4 kilobytes).</p>
</attribute>
+ <attribute name="parseBodyMethods" required="false">
+ <p>A comma-separated list of HTTP methods for which request
+ bodies will be parsed for request parameters identically
+ to POST. This is useful in RESTful applications that want to
+ support POST-style semantics for PUT requests.
+ Note that any setting other than <code>POST</code> causes Tomcat
+ to behave in a way that violates the servlet specification.
+ The HTTP method TRACE is specifically forbidden here in accordance
+ with the HTTP specification.
+ The default is <code>POST</code></p>
+ </attribute>
+
<attribute name="port" required="true">
<p>The TCP port number on which this <strong>Connector</strong>
will create a server socket and await incoming connections. Your
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1041892 - in /tomcat/trunk: java/org/apache/catalina/connector/
test/org/apache/catalina/connector/ webapps/docs/ webapps/docs/config/
Posted by Mark Thomas <ma...@apache.org>.
On 08/12/2010 18:03, Christopher Schultz wrote:
> Mark,
>
> On 12/5/2010 12:31 PM, Mark Thomas wrote:
>> On 03/12/2010 16:07, schultz@apache.org wrote:
>>> Author: schultz
>>> Date: Fri Dec 3 16:07:50 2010
>>> New Revision: 1041892
>>>
>>> + if(methodSet.contains("TRACE"))
>>> + throw new IllegalArgumentException("TRACE method MUST NOT include an entity (see RFC 2616 Section 9.6)");
>> This should use the StringManager for i18n support.
>
> Okay, last question: I can handle the English version... should I leave
> it at that and ask for a human translation, or should I use Google
> Translate to translate to the other existing locales in the LocalStrings
> files?
We usually just do the English and wait for contributions for the other
languages.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1041892 - in /tomcat/trunk: java/org/apache/catalina/connector/
test/org/apache/catalina/connector/ webapps/docs/ webapps/docs/config/
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 12/5/2010 12:31 PM, Mark Thomas wrote:
> On 03/12/2010 16:07, schultz@apache.org wrote:
>> Author: schultz
>> Date: Fri Dec 3 16:07:50 2010
>> New Revision: 1041892
>>
>> + if(methodSet.contains("TRACE"))
>> + throw new IllegalArgumentException("TRACE method MUST NOT include an entity (see RFC 2616 Section 9.6)");
> This should use the StringManager for i18n support.
Okay, last question: I can handle the English version... should I leave
it at that and ask for a human translation, or should I use Google
Translate to translate to the other existing locales in the LocalStrings
files?
Thanks,
-chris
Re: svn commit: r1041892 - in /tomcat/trunk: java/org/apache/catalina/connector/
test/org/apache/catalina/connector/ webapps/docs/ webapps/docs/config/
Posted by Mark Thomas <ma...@apache.org>.
On 08/12/2010 15:39, Christopher Schultz wrote:
> Mark,
>
> On 12/5/2010 12:31 PM, Mark Thomas wrote:
>> On 03/12/2010 16:07, schultz@apache.org wrote:
>>> Author: schultz
>>> Date: Fri Dec 3 16:07:50 2010
>>> New Revision: 1041892
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1041892&view=rev
>>> Log:
>>> Fixed bug 48692: Provide option to parse application/x-www-form-urlencoded PUT requests
>>
>> Some minor comments in-line.
>>
>>> + public boolean isParseBodyMethod(String method)
>> This method could (should?) be protected rather then public.
>
> While fixing the other issues, I was thinking about a few things. Nearly
> all methods in the Connector class are public, not protected. The
> mutators are the only dangerous methods and they must be public so that
> the Digester can set them during Connector configuration.
>
> Is there a compelling reason to make the isParseBodyMethod method
> protected? Or, is that simply considered an implementation detail that
> should be hidden from the public API?
Just a general principal not to expose stuff to the public API that
doesn't have to be. It is also easy to change protected to public later,
harder to go the other way.
One of the many things on my to-do list is to clean up the dependencies
between the modules, only expose the stuff that needs to be exposed and
expose everything through interfaces. Given the length of my to-do list
and the rate I am making progress I suspect that will happen sometime
around Tomcat 12 :)
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1041892 - in /tomcat/trunk: java/org/apache/catalina/connector/
test/org/apache/catalina/connector/ webapps/docs/ webapps/docs/config/
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 12/5/2010 12:31 PM, Mark Thomas wrote:
> On 03/12/2010 16:07, schultz@apache.org wrote:
>> Author: schultz
>> Date: Fri Dec 3 16:07:50 2010
>> New Revision: 1041892
>>
>> URL: http://svn.apache.org/viewvc?rev=1041892&view=rev
>> Log:
>> Fixed bug 48692: Provide option to parse application/x-www-form-urlencoded PUT requests
>
> Some minor comments in-line.
>
>> + public boolean isParseBodyMethod(String method)
> This method could (should?) be protected rather then public.
While fixing the other issues, I was thinking about a few things. Nearly
all methods in the Connector class are public, not protected. The
mutators are the only dangerous methods and they must be public so that
the Digester can set them during Connector configuration.
Is there a compelling reason to make the isParseBodyMethod method
protected? Or, is that simply considered an implementation detail that
should be hidden from the public API?
I'm open to either strategy.
Thanks,
-chris
Re: svn commit: r1041892 - in /tomcat/trunk: java/org/apache/catalina/connector/
test/org/apache/catalina/connector/ webapps/docs/ webapps/docs/config/
Posted by Mark Thomas <ma...@apache.org>.
On 05/12/2010 19:12, Christopher Schultz wrote:
> Mark,
>
> Thanks for the feedback. Is is better form to back-out these changes and
> apply a new patch, or to simply make these changes to trunk and commit them?
Just committing the fixes should be fine.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1041892 - in /tomcat/trunk: java/org/apache/catalina/connector/
test/org/apache/catalina/connector/ webapps/docs/ webapps/docs/config/
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
Thanks for the feedback. Is is better form to back-out these changes and
apply a new patch, or to simply make these changes to trunk and commit them?
Thanks,
-chris
On 12/5/2010 12:31 PM, Mark Thomas wrote:
> On 03/12/2010 16:07, schultz@apache.org wrote:
>> Author: schultz
>> Date: Fri Dec 3 16:07:50 2010
>> New Revision: 1041892
>>
>> URL: http://svn.apache.org/viewvc?rev=1041892&view=rev
>> Log:
>> Fixed bug 48692: Provide option to parse application/x-www-form-urlencoded PUT requests
>
> Some minor comments in-line.
>
>> + protected HashSet parseBodyMethodsSet;
> This needs to use generics (same for subsequent use later on in the class).
>
>> + public String getParseBodyMethods()
>> + {
>> + return (this.parseBodyMethods);
>> + }
> The Tomcat code style is to have brackets at the end of the previous line.
>
>> + if(methodSet.contains("TRACE"))
>> + throw new IllegalArgumentException("TRACE method MUST NOT include an entity (see RFC 2616 Section 9.6)");
> This should use the StringManager for i18n support.
>
>> + public boolean isParseBodyMethod(String method)
> This method could (should?) be protected rather then public.
>
>> - if (!getMethod().equalsIgnoreCase("POST"))
>> + if(!getConnector().isParseBodyMethod(getMethod()))
> The Tomcat code style is to have a space after the if.
>
>> <changelog>
>> + <update>
>> + <bug>48692</bug>: Provide option to parse
>> + <code>application/x-www-form-urlencoded</code> PUT requests. (schultz)
>> + </update>
>> <fix>
>> <bug>8705</bug>: <code>org.apache.catalina.SessionListener</code> now
>> extends <code>java.util.EventListener</code>. (markt)
> Bugs get added to the changelog in ascending numerical order within the
> appropriate section.
>
>> + <attribute name="parseBodyMethods" required="false">
>> + <p>A comma-separated list of HTTP methods for which request
>> + bodies will be parsed for request parameters identically
>> + to POST. This is useful in RESTful applications that want to
>> + support POST-style semantics for PUT requests.
>> + Note that any setting other than <code>POST</code> causes Tomcat
>> + to behave in a way that violates the servlet specification.
>> + The HTTP method TRACE is specifically forbidden here in accordance
>> + with the HTTP specification.
>> + The default is <code>POST</code></p>
>> + </attribute>
> "violates" is probably too strong a term here. There is some wiggle
> room in the language. I would suggest "goes against the intent" is
> probably closer.
>
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
Re: svn commit: r1041892 - in /tomcat/trunk: java/org/apache/catalina/connector/
test/org/apache/catalina/connector/ webapps/docs/ webapps/docs/config/
Posted by Mark Thomas <ma...@apache.org>.
On 05/12/2010 17:50, Caldarale, Charles R wrote:
>> From: Mark Thomas [mailto:markt@apache.org]
>> Subject: Re: svn commit: r1041892 - in /tomcat/trunk:
>> java/org/apache/catalina/connector/test/org/apache/
>> catalina/connector/ webapps/docs/ webapps/docs/config/
>
>>> + public String getParseBodyMethods()
>>> + {
>>> + return (this.parseBodyMethods);
>>> + }
>> The Tomcat code style is to have brackets at the end of the previous line.
>
> And there's no reason to put parentheses around a return value.
>
>>> + if(methodSet.contains("TRACE"))
>>> + throw new IllegalArgumentException("TRACE method MUST NOT
>>> include an entity (see RFC 2616 Section 9.6)");
>> This should use the StringManager for i18n support.
>
> Also needs a space after the if.
>
> Is there a convention for line length,
Max of 80 (not always adhered to).
> and how to handle long constant strings that aren't controlled by the StringManager?
Static finals with appropriate visibility depending on if they need to
be re-used.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1041892 - in /tomcat/trunk: java/org/apache/catalina/connector/
test/org/apache/catalina/connector/ webapps/docs/ webapps/docs/config/
Posted by Konstantin Kolinko <kn...@gmail.com>.
2010/12/5 Caldarale, Charles R <Ch...@unisys.com>:
> Is there a convention for line length,
The only strict rule is to use spaces, and never tabs. The rest is --
see how the file was formatted before.
I, personally, use "Java Conventions" with indent setting changed to
be spaces only, and let IDE format just the selected block of code. I
suspect that other do similar things as well.
It is preferred that the line does not exceed 80 chars.
> and how to handle long constant strings that aren't controlled by the StringManager?
Long strings could be splitted in parts by '+' (they will be
concatenated at compile time anyway), thought it would make harder to
search for them. Usually there is no need to bother. One example are
string constants in HTMLManagerServlet. Another is SecurityClassLoad.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
RE: svn commit: r1041892 - in /tomcat/trunk:
java/org/apache/catalina/connector/ test/org/apache/catalina/connector/
webapps/docs/ webapps/docs/config/
Posted by "Caldarale, Charles R" <Ch...@unisys.com>.
> From: Mark Thomas [mailto:markt@apache.org]
> Subject: Re: svn commit: r1041892 - in /tomcat/trunk:
> java/org/apache/catalina/connector/test/org/apache/
> catalina/connector/ webapps/docs/ webapps/docs/config/
> > + public String getParseBodyMethods()
> > + {
> > + return (this.parseBodyMethods);
> > + }
> The Tomcat code style is to have brackets at the end of the previous line.
And there's no reason to put parentheses around a return value.
> > + if(methodSet.contains("TRACE"))
> > + throw new IllegalArgumentException("TRACE method MUST NOT
> > include an entity (see RFC 2616 Section 9.6)");
> This should use the StringManager for i18n support.
Also needs a space after the if.
Is there a convention for line length, and how to handle long constant strings that aren't controlled by the StringManager?
- Chuck
THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers.
Re: svn commit: r1041892 - in /tomcat/trunk: java/org/apache/catalina/connector/
test/org/apache/catalina/connector/ webapps/docs/ webapps/docs/config/
Posted by Mark Thomas <ma...@apache.org>.
On 03/12/2010 16:07, schultz@apache.org wrote:
> Author: schultz
> Date: Fri Dec 3 16:07:50 2010
> New Revision: 1041892
>
> URL: http://svn.apache.org/viewvc?rev=1041892&view=rev
> Log:
> Fixed bug 48692: Provide option to parse application/x-www-form-urlencoded PUT requests
Some minor comments in-line.
> + protected HashSet parseBodyMethodsSet;
This needs to use generics (same for subsequent use later on in the class).
> + public String getParseBodyMethods()
> + {
> + return (this.parseBodyMethods);
> + }
The Tomcat code style is to have brackets at the end of the previous line.
> + if(methodSet.contains("TRACE"))
> + throw new IllegalArgumentException("TRACE method MUST NOT include an entity (see RFC 2616 Section 9.6)");
This should use the StringManager for i18n support.
> + public boolean isParseBodyMethod(String method)
This method could (should?) be protected rather then public.
> - if (!getMethod().equalsIgnoreCase("POST"))
> + if(!getConnector().isParseBodyMethod(getMethod()))
The Tomcat code style is to have a space after the if.
> <changelog>
> + <update>
> + <bug>48692</bug>: Provide option to parse
> + <code>application/x-www-form-urlencoded</code> PUT requests. (schultz)
> + </update>
> <fix>
> <bug>8705</bug>: <code>org.apache.catalina.SessionListener</code> now
> extends <code>java.util.EventListener</code>. (markt)
Bugs get added to the changelog in ascending numerical order within the
appropriate section.
> + <attribute name="parseBodyMethods" required="false">
> + <p>A comma-separated list of HTTP methods for which request
> + bodies will be parsed for request parameters identically
> + to POST. This is useful in RESTful applications that want to
> + support POST-style semantics for PUT requests.
> + Note that any setting other than <code>POST</code> causes Tomcat
> + to behave in a way that violates the servlet specification.
> + The HTTP method TRACE is specifically forbidden here in accordance
> + with the HTTP specification.
> + The default is <code>POST</code></p>
> + </attribute>
"violates" is probably too strong a term here. There is some wiggle
room in the language. I would suggest "goes against the intent" is
probably closer.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org