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 2023/06/23 16:45:05 UTC
[tomcat] branch main updated: Simplify reading of request body for x-www-form-urlencoded processing
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new e6ad02fb10 Simplify reading of request body for x-www-form-urlencoded processing
e6ad02fb10 is described below
commit e6ad02fb10506618f7e03c472105462b5c2f1d61
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Jun 23 17:44:36 2023 +0100
Simplify reading of request body for x-www-form-urlencoded processing
An incomplete body is the same as a client disconnect before the request
body has been read as that is the only way a client can provide an
incomplete body.
---
java/org/apache/catalina/connector/Request.java | 31 ++++++++++++++++++----
.../catalina/filters/FailedRequestFilter.java | 1 +
java/org/apache/tomcat/util/http/Parameters.java | 6 +++++
3 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java
index 312b3f4e81..88de9be19a 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -17,6 +17,7 @@
package org.apache.catalina.connector;
import java.io.BufferedReader;
+import java.io.EOFException;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
@@ -3115,10 +3116,7 @@ public class Request implements HttpServletRequest {
formData = new byte[len];
}
try {
- if (readPostBody(formData, len) != len) {
- parameters.setParseFailedReason(FailReason.REQUEST_BODY_INCOMPLETE);
- return;
- }
+ readPostBodyFully(formData, len);
} catch (IOException e) {
// Client disconnect
Context context = getContext();
@@ -3165,7 +3163,7 @@ public class Request implements HttpServletRequest {
/**
- * Read post body in an array.
+ * Read post body into an array.
*
* @param body The bytes array in which the body will be read
* @param len The body length
@@ -3173,7 +3171,10 @@ public class Request implements HttpServletRequest {
* @return the bytes count that has been read
*
* @throws IOException if an IO exception occurred
+ *
+ * @deprecated Unused. Will be removed in Tomcat 11.0.x onwards. Use {@link #readPostBodyFully(byte[], int)}
*/
+ @Deprecated
protected int readPostBody(byte[] body, int len) throws IOException {
int offset = 0;
@@ -3189,6 +3190,26 @@ public class Request implements HttpServletRequest {
}
+ /**
+ * Read post body into an array.
+ *
+ * @param body The bytes array in which the body will be read
+ * @param len The body length
+ *
+ * @throws IOException if an IO exception occurred or EOF is reached before the body has been fully read
+ */
+ protected void readPostBodyFully(byte[] body, int len) throws IOException {
+ int offset = 0;
+ do {
+ int inputLen = getStream().read(body, offset, len - offset);
+ if (inputLen <= 0) {
+ throw new EOFException();
+ }
+ offset += inputLen;
+ } while ((len - offset) > 0);
+ }
+
+
/**
* Read chunked post body.
*
diff --git a/java/org/apache/catalina/filters/FailedRequestFilter.java b/java/org/apache/catalina/filters/FailedRequestFilter.java
index 3e6368faf7..389575c7ce 100644
--- a/java/org/apache/catalina/filters/FailedRequestFilter.java
+++ b/java/org/apache/catalina/filters/FailedRequestFilter.java
@@ -49,6 +49,7 @@ public class FailedRequestFilter extends FilterBase {
return log;
}
+ @SuppressWarnings("deprecation")
@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
throws IOException, ServletException {
diff --git a/java/org/apache/tomcat/util/http/Parameters.java b/java/org/apache/tomcat/util/http/Parameters.java
index e56793b3de..29a6bc0f91 100644
--- a/java/org/apache/tomcat/util/http/Parameters.java
+++ b/java/org/apache/tomcat/util/http/Parameters.java
@@ -503,6 +503,12 @@ public final class Parameters {
IO_ERROR,
NO_NAME,
POST_TOO_LARGE,
+ /**
+ * Same as {@link #CLIENT_DISCONNECT}.
+ *
+ * @deprecated Unused. Will be removed in Tomcat 11.0.x onwards
+ */
+ @Deprecated
REQUEST_BODY_INCOMPLETE,
TOO_MANY_PARAMETERS,
UNKNOWN,
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch main updated: Simplify reading of request body for x-www-form-urlencoded processing
Posted by Mark Thomas <ma...@apache.org>.
On 23/06/2023 21:04, Christopher Schultz wrote:
> Mark,
>
> On 6/23/23 12:45, markt@apache.org wrote:
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> markt pushed a commit to branch main
>> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>>
>>
>> The following commit(s) were added to refs/heads/main by this push:
>> new e6ad02fb10 Simplify reading of request body for
>> x-www-form-urlencoded processing
>> e6ad02fb10 is described below
>>
>> commit e6ad02fb10506618f7e03c472105462b5c2f1d61
>> Author: Mark Thomas <ma...@apache.org>
>> AuthorDate: Fri Jun 23 17:44:36 2023 +0100
>>
>> Simplify reading of request body for x-www-form-urlencoded
>> processing
>> An incomplete body is the same as a client disconnect before the
>> request
>> body has been read as that is the only way a client can provide an
>> incomplete body.
>> ---
>> java/org/apache/catalina/connector/Request.java | 31
>> ++++++++++++++++++----
>> .../catalina/filters/FailedRequestFilter.java | 1 +
>> java/org/apache/tomcat/util/http/Parameters.java | 6 +++++
>> 3 files changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/java/org/apache/catalina/connector/Request.java
>> b/java/org/apache/catalina/connector/Request.java
>> index 312b3f4e81..88de9be19a 100644
>> --- a/java/org/apache/catalina/connector/Request.java
>> +++ b/java/org/apache/catalina/connector/Request.java
>> @@ -17,6 +17,7 @@
>> package org.apache.catalina.connector;
>> import java.io.BufferedReader;
>> +import java.io.EOFException;
>> import java.io.File;
>> import java.io.IOException;
>> import java.io.InputStream;
>> @@ -3115,10 +3116,7 @@ public class Request implements
>> HttpServletRequest {
>> formData = new byte[len];
>> }
>> try {
>> - if (readPostBody(formData, len) != len) {
>> -
>> parameters.setParseFailedReason(FailReason.REQUEST_BODY_INCOMPLETE);
>> - return;
>> - }
>> + readPostBodyFully(formData, len);
>> } catch (IOException e) {
>> // Client disconnect
>> Context context = getContext();
>> @@ -3165,7 +3163,7 @@ public class Request implements
>> HttpServletRequest {
>> /**
>> - * Read post body in an array.
>> + * Read post body into an array.
>> *
>> * @param body The bytes array in which the body will be read
>> * @param len The body length
>> @@ -3173,7 +3171,10 @@ public class Request implements
>> HttpServletRequest {
>> * @return the bytes count that has been read
>> *
>> * @throws IOException if an IO exception occurred
>> + *
>> + * @deprecated Unused. Will be removed in Tomcat 11.0.x onwards.
>> Use {@link #readPostBodyFully(byte[], int)}
>> */
>> + @Deprecated
>> protected int readPostBody(byte[] body, int len) throws
>> IOException {
>> int offset = 0;
>> @@ -3189,6 +3190,26 @@ public class Request implements
>> HttpServletRequest {
>> }
>> + /**
>> + * Read post body into an array.
>> + *
>> + * @param body The bytes array in which the body will be read
>> + * @param len The body length
>> + *
>> + * @throws IOException if an IO exception occurred or EOF is
>> reached before the body has been fully read
>> + */
>> + protected void readPostBodyFully(byte[] body, int len) throws
>> IOException {
>> + int offset = 0;
>> + do {
>> + int inputLen = getStream().read(body, offset, len - offset);
>
> Is it worth caching the return value of getStream() locally?
My instinct is no but I have no evidence to back up that instinct.
> I'd expect
> the JVM to inline that method after a while, but it's not trivial and
> could potentially generate a new CoyoteInputStream object every time
> through the loop.
>
> Sorry... I've been looking at C code lately and repeated function calls
> feel icky to me again.
No worries. It is a reasonable question. Given it has been this way for
a long time and I'm not aware of any issues I intend to leave it as is.
If a benchmark shows we'd be better changing it then I wouldn't object.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch main updated: Simplify reading of request body for x-www-form-urlencoded processing
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 6/23/23 12:45, markt@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch main
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/main by this push:
> new e6ad02fb10 Simplify reading of request body for x-www-form-urlencoded processing
> e6ad02fb10 is described below
>
> commit e6ad02fb10506618f7e03c472105462b5c2f1d61
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Fri Jun 23 17:44:36 2023 +0100
>
> Simplify reading of request body for x-www-form-urlencoded processing
>
> An incomplete body is the same as a client disconnect before the request
> body has been read as that is the only way a client can provide an
> incomplete body.
> ---
> java/org/apache/catalina/connector/Request.java | 31 ++++++++++++++++++----
> .../catalina/filters/FailedRequestFilter.java | 1 +
> java/org/apache/tomcat/util/http/Parameters.java | 6 +++++
> 3 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java
> index 312b3f4e81..88de9be19a 100644
> --- a/java/org/apache/catalina/connector/Request.java
> +++ b/java/org/apache/catalina/connector/Request.java
> @@ -17,6 +17,7 @@
> package org.apache.catalina.connector;
>
> import java.io.BufferedReader;
> +import java.io.EOFException;
> import java.io.File;
> import java.io.IOException;
> import java.io.InputStream;
> @@ -3115,10 +3116,7 @@ public class Request implements HttpServletRequest {
> formData = new byte[len];
> }
> try {
> - if (readPostBody(formData, len) != len) {
> - parameters.setParseFailedReason(FailReason.REQUEST_BODY_INCOMPLETE);
> - return;
> - }
> + readPostBodyFully(formData, len);
> } catch (IOException e) {
> // Client disconnect
> Context context = getContext();
> @@ -3165,7 +3163,7 @@ public class Request implements HttpServletRequest {
>
>
> /**
> - * Read post body in an array.
> + * Read post body into an array.
> *
> * @param body The bytes array in which the body will be read
> * @param len The body length
> @@ -3173,7 +3171,10 @@ public class Request implements HttpServletRequest {
> * @return the bytes count that has been read
> *
> * @throws IOException if an IO exception occurred
> + *
> + * @deprecated Unused. Will be removed in Tomcat 11.0.x onwards. Use {@link #readPostBodyFully(byte[], int)}
> */
> + @Deprecated
> protected int readPostBody(byte[] body, int len) throws IOException {
>
> int offset = 0;
> @@ -3189,6 +3190,26 @@ public class Request implements HttpServletRequest {
> }
>
>
> + /**
> + * Read post body into an array.
> + *
> + * @param body The bytes array in which the body will be read
> + * @param len The body length
> + *
> + * @throws IOException if an IO exception occurred or EOF is reached before the body has been fully read
> + */
> + protected void readPostBodyFully(byte[] body, int len) throws IOException {
> + int offset = 0;
> + do {
> + int inputLen = getStream().read(body, offset, len - offset);
Is it worth caching the return value of getStream() locally? I'd expect
the JVM to inline that method after a while, but it's not trivial and
could potentially generate a new CoyoteInputStream object every time
through the loop.
Sorry... I've been looking at C code lately and repeated function calls
feel icky to me again.
-chris
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org