Remaining Tomcat 10 items

2020-03-23 Thread Rémy Maucherat 
Hi,

I'm looking at the TODO list, in addition to some extra items. In order ...

- Java 11.
I suppose Jakarta EE 9 will require Java 11, is Java 11 [going to be]
required for Tomcat 10 ? It could be better to do it in 10.1.

- Remove the use of system properties to control configuration wherever
possible.
I still don't see the point for quite a few of them. For others however,
using sys props was a mistake, example the facade recycling. Also, the
digester can now pull from system and env properties, giving full
flexibility. Also also, this is a handy way to do things in cloud. I think
we should target the ones which should make sense.

- Reduce instances of setters and getters for the same property existing on
an object and its parent. This may require new objects to be exposed via
JMX.
Fixed enough ?

- Remove APR connector.
Is there still general approval for that, and is that still the plan for
Tomcat 10.0 ?

- Clean-up content-type header processing.
No opinion on that.

- RFC 3986 states (section 2.2) that a %nn encoded delimiter is NOT
equivalent to the decoded form. Provide an option not to decode delimiters
in %nn form.
Mark is working on it.

- Refactor DefaultServlet to use Ranges in parseRanges().
I looked at it and I don't get it.
In particular the Content-Ranges header (
https://tools.ietf.org/html/rfc7233#section-4.2) that uses
https://github.com/apache/tomcat/blob/master/java/org/apache/tomcat/util/http/parser/ContentRange.java#L75
The parser here requires a '=' but that should be a space in content-range
(ranges uses the '=' however), the example given in the spec is
"Content-Range: bytes 42-1233/1234".
Just wondering before starting to refactor.

Rémy

Re: Remaining Tomcat 10 items

2020-03-23 Thread Mark Thomas 
On 23/03/2020 09:37, Rémy Maucherat wrote:
> Hi,
> 
> I'm looking at the TODO list, in addition to some extra items. In order ...
> 
> - Java 11.
> I suppose Jakarta EE 9 will require Java 11, is Java 11 [going to be]
> required for Tomcat 10 ? It could be better to do it in 10.1.

No. It will be Java 8.

> - Remove the use of system properties to control configuration wherever
> possible.
> I still don't see the point for quite a few of them. For others however,
> using sys props was a mistake, example the facade recycling. Also, the
> digester can now pull from system and env properties, giving full
> flexibility. Also also, this is a handy way to do things in cloud. I
> think we should target the ones which should make sense.

For "wherever possible" read "where it makes sense to do so". My
intention was to target those system properties that were targetted at a
specific component and could/should be configured there.

> - Reduce instances of setters and getters for the same property existing
> on an object and its parent. This may require new objects to be exposed
> via JMX.
> Fixed enough ?

>From memory, this was primarily on the Connectors. That is certainly a
lot better. I'd like to review things before closing this one out.

> - Remove APR connector.
> Is there still general approval for that, and is that still the plan for
> Tomcat 10.0 ?

It feels like a big step but I do think this is the right thing to do.

> - Clean-up content-type header processing.
> No opinion on that.

It fixes an edge case bug. I have a plan for this. As long as it is done
before things go stable there is no rush.

> - RFC 3986 states (section 2.2) that a %nn encoded delimiter is NOT
> equivalent to the decoded form. Provide an option not to decode
> delimiters in %nn form.
> Mark is working on it.

I'm close to a patch for this but I got distracted with HTTP/0.9 issues.

> - Refactor DefaultServlet to use Ranges in parseRanges().
> I looked at it and I don't get it.
> In particular the Content-Ranges header
> (https://tools.ietf.org/html/rfc7233#section-4.2) that uses
> https://github.com/apache/tomcat/blob/master/java/org/apache/tomcat/util/http/parser/ContentRange.java#L75
> The parser here requires a '=' but that should be a space in
> content-range (ranges uses the '=' however), the example given in the
> spec is "Content-Range: bytes 42-1233/1234".
> Just wondering before starting to refactor.

That is the response header, not the request header. Take a look at
sections 2.1 and 3.1

Mark

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

[Bug 64210] parsing request headers fail

2020-03-23 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64210

--- Comment #9 from Xing  ---
I don't think the current solution resolved all the header issues.
For example, our project use Socket TCP to monitor Tomcat status like this:
"get /platform HTTP/1.0\n\n" (double \n, no \r) 

Before version 8.5.50 (same for 9.0.30), it response 302 like:
2020-03-09 21:17:12.000 CST DEBUG [18950:140054285395776] childHeartbeat: child
reply=HTTP/1.1 302 ^M
Location: http://localhost:8080/platform/^M
Date: Mon, 09 Mar 2020 13:17:12 GMT^M
Connection: close^M
Server: This information has been blocked for security reasons^M
^M

With 8.5.51 (9.0.31), it no response and make my monitor always wrongly restart
Tomcat.

With 8.5.53 (9.0.33), it response 400 like:
2020-03-23 18:58:45.000 CST DEBUG [8584:140509820770112] childHeartbeat: child
reply=HTTP/1.1 400 ^M
Content-Type: text/html;charset=utf-8^M
Content-Language: en^M
Content-Length: 1823^M
Date: Mon, 23 Mar 2020 10:58:45 GMT^M
Connection: close^M
Server: This information has been blocked for security reasons^M
^M
HTTP Status 400 – Bad
Request

[Bug 64210] parsing request headers fail

2020-03-23 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64210

--- Comment #10 from Mark Thomas  ---
(In reply to Xing from comment #9)
> I don't think the current solution resolved all the header issues.
> For example, our project use Socket TCP to monitor Tomcat status like this:
> "get /platform HTTP/1.0\n\n" (double \n, no \r) 

That request is invalid and is, therefore, rejected with a 400 response.

You can fix the issue by correcting the broken client.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[GitHub] [tomcat] martin-g commented on issue #264: fix regression: only skip jar-scanning completely if scanSet is empty

2020-03-23 Thread GitBox 
martin-g commented on issue #264: fix regression: only skip jar-scanning 
completely if scanSet is empty
URL: https://github.com/apache/tomcat/pull/264#issuecomment-602555158
 
 
   Fixed with 
https://github.com/apache/tomcat/commit/9c6563bf6ac723cda4e78d0e8c1a996fa1b8ce56


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] martin-g closed pull request #264: fix regression: only skip jar-scanning completely if scanSet is empty

2020-03-23 Thread GitBox 
martin-g closed pull request #264: fix regression: only skip jar-scanning 
completely if scanSet is empty
URL: https://github.com/apache/tomcat/pull/264
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] Iridias commented on issue #264: fix regression: only skip jar-scanning completely if scanSet is empty

2020-03-23 Thread GitBox 
Iridias commented on issue #264: fix regression: only skip jar-scanning 
completely if scanSet is empty
URL: https://github.com/apache/tomcat/pull/264#issuecomment-602564747
 
 
   > Fixed with 9c6563b
   
   Hm, that would ignore the case, that `jarsToScan` is specified but empty. 
(Because then `defaultScan` would be an empty String instead of null)
   Is that intended?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] rmaucher commented on issue #264: fix regression: only skip jar-scanning completely if scanSet is empty

2020-03-23 Thread GitBox 
rmaucher commented on issue #264: fix regression: only skip jar-scanning 
completely if scanSet is empty
URL: https://github.com/apache/tomcat/pull/264#issuecomment-602567135
 
 
   I hadn't seen this PR at all as there was a BZ there already and it didn't 
mention it.
   I will make the extra change (after all this is a stupid configuration, so 
it doesn't hurt to handle an even more stupid configuration) and will also give 
you credit since your patch was correct.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[tomcat] branch master updated: Check that scan set is empty as it is more accurate

2020-03-23 Thread remm 
This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
 new be649bc  Check that scan set is empty as it is more accurate
be649bc is described below

commit be649bcbb67d6d0bf1fb107e88cc8274fe0eb85e
Author: remm 
AuthorDate: Mon Mar 23 13:49:03 2020 +0100

Check that scan set is empty as it is more accurate

Credit for a PR I had missed from Iridias.
---
 java/org/apache/tomcat/util/scan/StandardJarScanFilter.java | 2 +-
 webapps/docs/changelog.xml  | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/tomcat/util/scan/StandardJarScanFilter.java 
b/java/org/apache/tomcat/util/scan/StandardJarScanFilter.java
index 280f21b..1451de4 100644
--- a/java/org/apache/tomcat/util/scan/StandardJarScanFilter.java
+++ b/java/org/apache/tomcat/util/scan/StandardJarScanFilter.java
@@ -43,8 +43,8 @@ public class StandardJarScanFilter implements JarScanFilter {
 defaultSkip = System.getProperty(Constants.SKIP_JARS_PROPERTY);
 populateSetFromAttribute(defaultSkip, defaultSkipSet);
 defaultScan = System.getProperty(Constants.SCAN_JARS_PROPERTY);
-defaultSkipAll = (defaultSkipSet.contains("*") || 
defaultSkipSet.contains("*.jar")) && defaultScan == null;
 populateSetFromAttribute(defaultScan, defaultScanSet);
+defaultSkipAll = (defaultSkipSet.contains("*") || 
defaultSkipSet.contains("*.jar")) && defaultScanSet.isEmpty();
 }
 
 private String tldSkip;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b90111b..cc6269d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -63,7 +63,8 @@
   
   
 64247: Using a wildcard for jarsToSkip should
-not override a possibly present jarsToScan. (remm)
+not override a possibly present jarsToScan. Based on code
+submitted by Iridias. (remm)
   
 
   


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

[Bug 64210] parsing request headers fail

2020-03-23 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64210

--- Comment #11 from Xing  ---
(In reply to Mark Thomas from comment #10)
> (In reply to Xing from comment #9)
> > I don't think the current solution resolved all the header issues.
> > For example, our project use Socket TCP to monitor Tomcat status like this:
> > "get /platform HTTP/1.0\n\n" (double \n, no \r) 
> 
> That request is invalid and is, therefore, rejected with a 400 response.
> 
> You can fix the issue by correcting the broken client.

No configuration to resolve this?
Then should I remove the "\n" in my request?

Thanks
Xing

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Request line parsing

2020-03-23 Thread Mark Thomas 
Hi,

I am currently looking at the request line parsing. I'll try and set out
each issue in turn.

End of line parsing
===

Prior to the recent changes, Tomcat allowed CRLF or LF to mark the end
of a line. The unwanted side effect was that CR could appear in the
header value. This caused problems and was tightened up to only allow
CRLF as a line terminator.

Currently Tomcat requires CRLF everywhere apart from the end of the
request line for a HTTP 0.9 request where it also allows LF.

This requirement to accept just LF as a line terminator first emerged in
the W3C spec [1]. RFC 1945 [2] and RFC 2616 [3] retained this as a
recommendation for all line terminators, RFC 7230 [4] no longer includes
this recommendation.

RFC 7230 also removes the expectation that a server that supports
HTTP/1.1 will support HTTP 0.9.

Arguably the current spec for HTTP/0.9 is [3].

The Servlet spec references RFC 7230 and RFC 1945 so arguably HTTP/0.9
support is expected.


SP vs whitespace


Tomcat currently accepts any combination of SP and HTAB where RFC 7230
calls for a single SP. This stems from a recommendation in RFC 2616
which is no longer present in RFC 7230.


I think we have three options.

1. No changes.
   CRLF is required everywhere apart from HTTP/0.9 where LF is also
   accepted.
   Any combination of SP/HTAB is accepted where SP is required.

2. Tighten up as per RFC 7230
   a) Require CRLF for all line endings
   b) Require SP where specified
   c) Drop HTTP/0.9 support

3. Relax the recent changes to allow CRLF or LF as a line terminator
   everywhere without allowing CR to appear in a request header.

I think we should follow 1) for Tomcat 7, 8 & 9.

I'm leaning towards 1 for 10.0.x as well with a view to discussing 2 in
the Servlet project. i.e. explicitly dropping HTTP 0.9 support and the
"Tolerant applications" requirements of RFC 1945 for Jakarta EE 10
(Tomcat 10.1.x).

In short this means largely do nothing apart from may be adding a few
more tests to explicitly check behaviour for various edge cases.

I'll note that the regressions reported with the recent change to
requiring CRLF as a line terminator caused issues with valid HTTP/0.9
requests but that this should now be resolved.

We have had one user issue reported where a custom client was using LFLF
as a line terminator and requests are now being rejected. Given that was
never valid, I'm OK with that.

Thoughts?

Mark



[1] https://www.w3.org/Protocols/HTTP/AsImplemented.html
[2] https://tools.ietf.org/html/rfc1945
[3] https://tools.ietf.org/html/rfc2616
[4] https://tools.ietf.org/html/rfc7230





With all of the above in mind I propose:

- Doing nothing! I think Tomcat is striking the right balance here.

This means:
GET /CRLF   -> processed as HTTP/0.9
GET /LF -> processed as HTTP/0.9
GET / CRLF  -> processed as HTTP/1.1 and rejected as invalid
GET / LF-> processed as HTTP/1.1 and rejected as invalid

I want to write some tests to check this is behaving as expected but I'm
not expecting any changes to the parsing at this point.

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

[tomcat] branch 8.5.x updated: Check that scan set is empty as it is more accurate

2020-03-23 Thread remm 
This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/8.5.x by this push:
 new 6582703  Check that scan set is empty as it is more accurate
6582703 is described below

commit 658270316f5789cb016eb41149ced52f2b9c943c
Author: remm 
AuthorDate: Mon Mar 23 13:49:03 2020 +0100

Check that scan set is empty as it is more accurate

Credit for a PR I had missed from Iridias.
---
 java/org/apache/tomcat/util/scan/StandardJarScanFilter.java | 2 +-
 webapps/docs/changelog.xml  | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/tomcat/util/scan/StandardJarScanFilter.java 
b/java/org/apache/tomcat/util/scan/StandardJarScanFilter.java
index 9167e1f..a77d6b1 100644
--- a/java/org/apache/tomcat/util/scan/StandardJarScanFilter.java
+++ b/java/org/apache/tomcat/util/scan/StandardJarScanFilter.java
@@ -43,8 +43,8 @@ public class StandardJarScanFilter implements JarScanFilter {
 defaultSkip = System.getProperty(Constants.SKIP_JARS_PROPERTY);
 populateSetFromAttribute(defaultSkip, defaultSkipSet);
 defaultScan = System.getProperty(Constants.SCAN_JARS_PROPERTY);
-defaultSkipAll = (defaultSkipSet.contains("*") || 
defaultSkipSet.contains("*.jar")) && defaultScan == null;
 populateSetFromAttribute(defaultScan, defaultScanSet);
+defaultSkipAll = (defaultSkipSet.contains("*") || 
defaultSkipSet.contains("*.jar")) && defaultScanSet.isEmpty();
 }
 
 private String tldSkip;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index f378c11..e49f464 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -63,7 +63,8 @@
   
   
 64247: Using a wildcard for jarsToSkip should
-not override a possibly present jarsToScan. (remm)
+not override a possibly present jarsToScan. Based on code
+submitted by Iridias. (remm)
   
 
   


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

[tomcat] branch 9.0.x updated: Check that scan set is empty as it is more accurate

2020-03-23 Thread remm 
This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
 new 304c72a  Check that scan set is empty as it is more accurate
304c72a is described below

commit 304c72abbad33a4d074e56717f6c00dd97289105
Author: remm 
AuthorDate: Mon Mar 23 13:49:03 2020 +0100

Check that scan set is empty as it is more accurate

Credit for a PR I had missed from Iridias.
---
 java/org/apache/tomcat/util/scan/StandardJarScanFilter.java | 2 +-
 webapps/docs/changelog.xml  | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/tomcat/util/scan/StandardJarScanFilter.java 
b/java/org/apache/tomcat/util/scan/StandardJarScanFilter.java
index 280f21b..1451de4 100644
--- a/java/org/apache/tomcat/util/scan/StandardJarScanFilter.java
+++ b/java/org/apache/tomcat/util/scan/StandardJarScanFilter.java
@@ -43,8 +43,8 @@ public class StandardJarScanFilter implements JarScanFilter {
 defaultSkip = System.getProperty(Constants.SKIP_JARS_PROPERTY);
 populateSetFromAttribute(defaultSkip, defaultSkipSet);
 defaultScan = System.getProperty(Constants.SCAN_JARS_PROPERTY);
-defaultSkipAll = (defaultSkipSet.contains("*") || 
defaultSkipSet.contains("*.jar")) && defaultScan == null;
 populateSetFromAttribute(defaultScan, defaultScanSet);
+defaultSkipAll = (defaultSkipSet.contains("*") || 
defaultSkipSet.contains("*.jar")) && defaultScanSet.isEmpty();
 }
 
 private String tldSkip;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b8e3630..37f52ae 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -63,7 +63,8 @@
   
   
 64247: Using a wildcard for jarsToSkip should
-not override a possibly present jarsToScan. (remm)
+not override a possibly present jarsToScan. Based on code
+submitted by Iridias. (remm)
   
 
   


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

Re: Request line parsing

2020-03-23 Thread Rémy Maucherat 
On Mon, Mar 23, 2020 at 2:01 PM Mark Thomas  wrote:

> Hi,
>
> I am currently looking at the request line parsing. I'll try and set out
> each issue in turn.
>
> End of line parsing
> ===
>
> Prior to the recent changes, Tomcat allowed CRLF or LF to mark the end
> of a line. The unwanted side effect was that CR could appear in the
> header value. This caused problems and was tightened up to only allow
> CRLF as a line terminator.
>
> Currently Tomcat requires CRLF everywhere apart from the end of the
> request line for a HTTP 0.9 request where it also allows LF.
>
> This requirement to accept just LF as a line terminator first emerged in
> the W3C spec [1]. RFC 1945 [2] and RFC 2616 [3] retained this as a
> recommendation for all line terminators, RFC 7230 [4] no longer includes
> this recommendation.
>
> RFC 7230 also removes the expectation that a server that supports
> HTTP/1.1 will support HTTP 0.9.
>
> Arguably the current spec for HTTP/0.9 is [3].
>
> The Servlet spec references RFC 7230 and RFC 1945 so arguably HTTP/0.9
> support is expected.
>
>
> SP vs whitespace
> 
>
> Tomcat currently accepts any combination of SP and HTAB where RFC 7230
> calls for a single SP. This stems from a recommendation in RFC 2616
> which is no longer present in RFC 7230.
>
>
> I think we have three options.
>
> 1. No changes.
>CRLF is required everywhere apart from HTTP/0.9 where LF is also
>accepted.
>Any combination of SP/HTAB is accepted where SP is required.
>
> 2. Tighten up as per RFC 7230
>a) Require CRLF for all line endings
>b) Require SP where specified
>c) Drop HTTP/0.9 support
>
> 3. Relax the recent changes to allow CRLF or LF as a line terminator
>everywhere without allowing CR to appear in a request header.
>
> I think we should follow 1) for Tomcat 7, 8 & 9.
>
> I'm leaning towards 1 for 10.0.x as well with a view to discussing 2 in
> the Servlet project. i.e. explicitly dropping HTTP 0.9 support and the
> "Tolerant applications" requirements of RFC 1945 for Jakarta EE 10
> (Tomcat 10.1.x).
>
> In short this means largely do nothing apart from may be adding a few
> more tests to explicitly check behaviour for various edge cases.
>
> I'll note that the regressions reported with the recent change to
> requiring CRLF as a line terminator caused issues with valid HTTP/0.9
> requests but that this should now be resolved.
>
> We have had one user issue reported where a custom client was using LFLF
> as a line terminator and requests are now being rejected. Given that was
> never valid, I'm OK with that.
>
> Thoughts?
>
> Mark
>
>
>
> [1] https://www.w3.org/Protocols/HTTP/AsImplemented.html
> [2] https://tools.ietf.org/html/rfc1945
> [3] https://tools.ietf.org/html/rfc2616
> [4] https://tools.ietf.org/html/rfc7230
>
>
>
>
>
> With all of the above in mind I propose:
>
> - Doing nothing! I think Tomcat is striking the right balance here.
>
> This means:
> GET /CRLF   -> processed as HTTP/0.9
> GET /LF -> processed as HTTP/0.9
> GET / CRLF  -> processed as HTTP/1.1 and rejected as invalid
> GET / LF-> processed as HTTP/1.1 and rejected as invalid
>
> I want to write some tests to check this is behaving as expected but I'm
> not expecting any changes to the parsing at this point.
>

+1, that sounds really good !

Rémy


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

[Bug 64210] parsing request headers fail

2020-03-23 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64210

--- Comment #12 from Mark Thomas  ---
The correct line terminator for an HTTP/1.0 request is CRLF ("/r/n"). You
should use that.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: Remaining Tomcat 10 items

2020-03-23 Thread Christopher Schultz 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Rémy,

On 3/23/20 05:37, Rémy Maucherat wrote:
> I'm looking at the TODO list, in addition to some extra items. In
> order ...
>
> - Remove APR connector. Is there still general approval for that,
> and is that still the plan for Tomcat 10.0 ?

See the thread "[PROPOSAL] Tomcat 10: Drop AJP Connector" from
2019-10-07 and later. All replies were in favor of dropping AJP
support, though there were not a large number of those replies.
(michael-o had a concern about some performance problem in the HTTP
connector(s) but it seems that was a regression that was corrected
immediately after it was reported.)

- -chris
-BEGIN PGP SIGNATURE-
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl54ytUACgkQHPApP6U8
pFj81A//QoeYRkzDCw/acYy1FOlrWGapDnvP/Am4sr4UDtcMjjLpGP7SyhMv2EOE
k5ndZ7xO+WFpzGVa13a5kzfrHGUt2m3mbZDMpkLsW2x55FUfq+Oe3g4e8g0Bjsmw
5zVwoOWuap+6wRN3dqNAQ3ZAPDBSz62xocY+daqNtmnKijmqyAMIxL4bqzRHaVQW
dpJeWRCQ2W4eg66594HSr59qfJ1XyiXH8jLYxEHvDA1+ilCOZC+hCBLvGO24KUx3
yJhc6ZbnQKhvb9W9tG1x8jls3H571ySno5dQ7YSFultv+dTtSNPL6dmS0stTCeNH
KvlJbwEip/ic1at3osr6Ua4Zq/IWSuedt7mRoXWx2M8SEldGTOY1HoM7ZB1jzcK+
25A46rq7knctJ1zDw5UXwL3s91qP7y1pwjCdX6UCfiu0XQg80VUXDKv1HiGH1aj1
lGxzG/INEios8SBHUpKKuXvDfd8Sm/z54VQSlvmBr817eX3ljdb1XR6o/Ld1KmiC
1eutQyZ2vIWBD3sR8BEum+9FvQ/XpINArSirC/DQVHP0TfInQlQq60dry4OjPFX2
delqCOM6pyVur9I362HKwgPXHHAGwdx8mWpC536nsNw3m0YMdW3dVImjbtz9nZ3H
GSeGziYslnKHAh8WmH5UxHJL59BXFTCTMUOOvIqsZkWDH3UuWmk=
=31Oo
-END PGP SIGNATURE-

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

[Bug 64210] parsing request headers fail

2020-03-23 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64210

--- Comment #13 from Xing  ---
(In reply to Mark Thomas from comment #12)
> The correct line terminator for an HTTP/1.0 request is CRLF ("/r/n"). You
> should use that.

Thanks a lot, I'm compiling :)

Xing

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[tomcat] branch master updated: Remove internal Range helpers

2020-03-23 Thread remm 
This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
 new b9aff64  Remove internal Range helpers
b9aff64 is described below

commit b9aff64f78740235a5565004423be40cadc740ac
Author: remm 
AuthorDate: Mon Mar 23 15:54:23 2020 +0100

Remove internal Range helpers

Refactor DefaultServlet to avoid using an internal Range structure that
is duplicated from the parsing result. Hopefully no regressions.
---
 TOMCAT-NEXT.txt|   2 -
 .../apache/catalina/servlets/DefaultServlet.java   | 172 ++---
 .../org/apache/tomcat/util/http/parser/Ranges.java |   2 +-
 webapps/docs/changelog.xml |   4 +
 4 files changed, 83 insertions(+), 97 deletions(-)

diff --git a/TOMCAT-NEXT.txt b/TOMCAT-NEXT.txt
index 0b3a519..f8cd94c 100644
--- a/TOMCAT-NEXT.txt
+++ b/TOMCAT-NEXT.txt
@@ -36,8 +36,6 @@ New items for 10.0.x onwards:
  3. RFC 3986 states (section 2.2) that a %nn encoded delimiter is NOT 
equivalent
 to the decoded form. Provide an option not to decode delimiters in %nn 
form.
 
- 4. Refactor DefaultServlet to use Ranges in parseRanges().
-
 Deferred until 10.1.x:
 
  1.  Remove the ExtensionValidator and associated classes (assuming that the
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java 
b/java/org/apache/catalina/servlets/DefaultServlet.java
index 2db2251..406b120 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -41,7 +41,6 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.Enumeration;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
 import java.util.StringTokenizer;
@@ -151,9 +150,9 @@ public class DefaultServlet extends HttpServlet {
 /**
  * Full range marker.
  */
-protected static final ArrayList FULL = new ArrayList<>();
+protected static final Ranges FULL = new Ranges(null, new 
ArrayList());
 
-private static final Range IGNORE = new Range();
+private static final ContentRange IGNORE = new ContentRange(null, 0, 0, 0);
 
 /**
  * MIME multipart separation string
@@ -614,7 +613,7 @@ public class DefaultServlet extends HttpServlet {
 
 WebResource resource = resources.getResource(path);
 
-Range range = parseContentRange(req, resp);
+ContentRange range = parseContentRange(req, resp);
 
 if (range == null) {
 // Processing error. parseContentRange() set the error code
@@ -666,7 +665,7 @@ public class DefaultServlet extends HttpServlet {
  * @return the associated file object
  * @throws IOException an IO error occurred
  */
-protected File executePartialPut(HttpServletRequest req, Range range,
+protected File executePartialPut(HttpServletRequest req, ContentRange 
range,
  String path)
 throws IOException {
 
@@ -703,10 +702,10 @@ public class DefaultServlet extends HttpServlet {
 }
 }
 
-randAccessContentFile.setLength(range.length);
+randAccessContentFile.setLength(range.getLength());
 
 // Append data in request input stream to contentFile
-randAccessContentFile.seek(range.start);
+randAccessContentFile.seek(range.getStart());
 int numBytesRead;
 byte[] transferBuffer = new byte[BUFFER_SIZE];
 try (BufferedInputStream requestBufInStream =
@@ -928,7 +927,7 @@ public class DefaultServlet extends HttpServlet {
 }
 }
 
-ArrayList ranges = FULL;
+Ranges ranges = FULL;
 long contentLength = -1L;
 
 if (resource.isDirectory()) {
@@ -1148,21 +1147,22 @@ public class DefaultServlet extends HttpServlet {
 
 } else {
 
-if ((ranges == null) || (ranges.isEmpty()))
+if ((ranges == null) || (ranges.getEntries().isEmpty()))
 return;
 
 // Partial content response.
 
 response.setStatus(HttpServletResponse.SC_PARTIAL_CONTENT);
 
-if (ranges.size() == 1) {
+if (ranges.getEntries().size() == 1) {
 
-Range range = ranges.get(0);
-response.addHeader("Content-Range", "bytes "
-   + range.start
-   + "-" + range.end + "/"
-   + range.length);
-long length = range.end - range.start + 1;
+Ranges.Entry range = ranges.getEntries().get(0);
+long resourceLength = resource.getContentLength();
+long start = getStart(range, resourceLength);
+long end = getEnd(range, re

Re: Request line parsing

2020-03-23 Thread Christopher Schultz 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Mark,

On 3/23/20 09:01, Mark Thomas wrote:
> Hi,
>
> I am currently looking at the request line parsing. I'll try and
> set out each issue in turn.
>
> End of line parsing ===
>
> Prior to the recent changes, Tomcat allowed CRLF or LF to mark the
> end of a line. The unwanted side effect was that CR could appear in
> the header value. This caused problems and was tightened up to only
> allow CRLF as a line terminator.
>
> Currently Tomcat requires CRLF everywhere apart from the end of
> the request line for a HTTP 0.9 request where it also allows LF.
>
> This requirement to accept just LF as a line terminator first
> emerged in the W3C spec [1]. RFC 1945 [2] and RFC 2616 [3] retained
> this as a recommendation for all line terminators, RFC 7230 [4] no
> longer includes this recommendation.
>
> RFC 7230 also removes the expectation that a server that supports
> HTTP/1.1 will support HTTP 0.9.
>
> Arguably the current spec for HTTP/0.9 is [3].
>
> The Servlet spec references RFC 7230 and RFC 1945 so arguably
> HTTP/0.9 support is expected.
>
>
> SP vs whitespace 
>
> Tomcat currently accepts any combination of SP and HTAB where RFC
> 7230 calls for a single SP. This stems from a recommendation in RFC
> 2616 which is no longer present in RFC 7230.
>
>
> I think we have three options.
>
> 1. No changes. CRLF is required everywhere apart from HTTP/0.9
> where LF is also accepted. Any combination of SP/HTAB is accepted
> where SP is required.
>
> 2. Tighten up as per RFC 7230 a) Require CRLF for all line endings
> b) Require SP where specified c) Drop HTTP/0.9 support
>
> 3. Relax the recent changes to allow CRLF or LF as a line
> terminator everywhere without allowing CR to appear in a request
> header.
>
> I think we should follow 1) for Tomcat 7, 8 & 9.

+1

> I'm leaning towards 1 for 10.0.x as well with a view to discussing
> 2 in the Servlet project. i.e. explicitly dropping HTTP 0.9 support
> and the "Tolerant applications" requirements of RFC 1945 for
> Jakarta EE 10 (Tomcat 10.1.x).
>
> In short this means largely do nothing apart from may be adding a
> few more tests to explicitly check behaviour for various edge
> cases.
>
> I'll note that the regressions reported with the recent change to
> requiring CRLF as a line terminator caused issues with valid
> HTTP/0.9 requests but that this should now be resolved.
>
> We have had one user issue reported where a custom client was using
> LFLF as a line terminator and requests are now being rejected.
> Given that was never valid, I'm OK with that.

+1

My only concern here is that request line + header-processing really
has to match whatever reverse proxy servers are doing as well, and
that's really not something we can know for sure. I don't think there
is a single safe implementation that will make everyone happy.

Is there a way to make this kind of thing pluggable with a few obvious
implementations so that users can select which one makes the most
sense for their environment? Similar to cookie-proessing, we could
have a super-spec-compliant one which always requires CRLF line
endings (which I guess means dropping support for HTTP 0.9), and is
super strict about everything else.

Another implementation could work the way Tomcat currently behaves,
being mostly spec-compliant and a little tolerant of some sloppiness.

This will allow an environment where e.g. httpd is in use to use one
implementation while Squid, IIS, nginx, etc. can make different
choices depending upon how the proxy will interpret things.

I realize that means more code. :( But if the proxy and origin server
disagree about how to interpret things, Bad Things can happen. If we
take a very strict interpretation of everything, I think we can
actually make the origin server safe, but we may break environments
which are relying on non-strict behavior.

- -chris
-BEGIN PGP SIGNATURE-
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl54zucACgkQHPApP6U8
pFglJQ//WFBVYUlgHIXU86AgtpquhrMcl7eTR/AAnyJ5K5SGEUxS9sKgDS+K5Xsu
p1j5FpRBxpA5RY3xzn4X30dj1ps17sIFfntcIydHqZDePtFB78J8THMIeonaMaQE
SO1enP7HH2unowYewBZGjKvdEkLyCnFWz9JXJDjIvnNeqSapEdOttpVhMs+/sSjZ
8yPoOVhTAmYhnlJAH856KvXb/JVfhbBuvYfppuGGnV7aj8iy83BgYrXgOk8DJN1Z
7VowjCyabcSqZGMgCaXcpph3UTaYkuKjV30PYEssgTqT4xtibA5cGt75m4OVAnIH
ahVIKohGs+j+t+aNYJZsXLk2AtMF051ZygdbhwyXl2+/eS2X4mmISu3gpswIS1u3
Sz36boLhLBkTii67SXXXwIe+0e2WyYl4UA3sD/xzTKFDUMXvn5bjATxSHsCsUAO7
fYXlYxzE2ZyK0EyZ9Qox3K+d72bC/y8CjJvLZo1BHvUtt0QrvhsA0dgS6gXV/qjZ
hPgL3kodFF3dm5QQKEMYSzG1b64zPbw76DQ7nc7DcwNnlEJEPmk6NfxNlLE9SoCA
/9s1RJwiY+LVDqocElWZp1nwHWVAbFsPh9mr3nT5T5P1PMKGTIySTevfSHeBxJBw
7ua5N9SUg6KDLm6EwnfPL9fqSDMJerDn+b4oMu19TjQoPBjTXQ8=
=VY+W
-END PGP SIGNATURE-

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

Re: Request line parsing

2020-03-23 Thread Michael Osipov 

Am 2020-03-23 um 14:01 schrieb Mark Thomas:

Hi,

I am currently looking at the request line parsing. I'll try and set out
each issue in turn.

End of line parsing
===

Prior to the recent changes, Tomcat allowed CRLF or LF to mark the end
of a line. The unwanted side effect was that CR could appear in the
header value. This caused problems and was tightened up to only allow
CRLF as a line terminator.

Currently Tomcat requires CRLF everywhere apart from the end of the
request line for a HTTP 0.9 request where it also allows LF.

This requirement to accept just LF as a line terminator first emerged in
the W3C spec [1]. RFC 1945 [2] and RFC 2616 [3] retained this as a
recommendation for all line terminators, RFC 7230 [4] no longer includes
this recommendation.

RFC 7230 also removes the expectation that a server that supports
HTTP/1.1 will support HTTP 0.9.

Arguably the current spec for HTTP/0.9 is [3].

The Servlet spec references RFC 7230 and RFC 1945 so arguably HTTP/0.9
support is expected.


SP vs whitespace


Tomcat currently accepts any combination of SP and HTAB where RFC 7230
calls for a single SP. This stems from a recommendation in RFC 2616
which is no longer present in RFC 7230.


I think we have three options.

1. No changes.
CRLF is required everywhere apart from HTTP/0.9 where LF is also
accepted.
Any combination of SP/HTAB is accepted where SP is required.

2. Tighten up as per RFC 7230
a) Require CRLF for all line endings
b) Require SP where specified
c) Drop HTTP/0.9 support

3. Relax the recent changes to allow CRLF or LF as a line terminator
everywhere without allowing CR to appear in a request header.

I think we should follow 1) for Tomcat 7, 8 & 9.

I'm leaning towards 1 for 10.0.x as well with a view to discussing 2 in
the Servlet project. i.e. explicitly dropping HTTP 0.9 support and the
"Tolerant applications" requirements of RFC 1945 for Jakarta EE 10
(Tomcat 10.1.x).


Makes sense for <= 9 and the evaluation for 10

M

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

Re: Remaining Tomcat 10 items

2020-03-23 Thread Michael Osipov 

Am 2020-03-23 um 15:42 schrieb Christopher Schultz:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Rémy,

On 3/23/20 05:37, Rémy Maucherat wrote:

I'm looking at the TODO list, in addition to some extra items. In
order ...

- Remove APR connector. Is there still general approval for that,
and is that still the plan for Tomcat 10.0 ?


See the thread "[PROPOSAL] Tomcat 10: Drop AJP Connector" from
2019-10-07 and later. All replies were in favor of dropping AJP
support, though there were not a large number of those replies.
(michael-o had a concern about some performance problem in the HTTP
connector(s) but it seems that was a regression that was corrected
immediately after it was reported.)


It was a misconfiguration of the tests on master. With Mark's test code 
I was able to find the minimal required buffer size for three operating 
systems.


I will run master tests on several OSes and OpenJDK versions again and 
compare numbers. If they are significantly different, I will report them.


M


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

Re: Remaining Tomcat 10 items

2020-03-23 Thread Michael Osipov 

Am 2020-03-23 um 10:37 schrieb Rémy Maucherat:

Hi,

I'm looking at the TODO list, in addition to some extra items. In order ...


You missed one point:

Remove deprecated code marked for removal in Tomcat 10.

M


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

Re: Request line parsing

2020-03-23 Thread Mark Thomas 
On 23/03/2020 14:59, Christopher Schultz wrote:



> My only concern here is that request line + header-processing really
> has to match whatever reverse proxy servers are doing as well, and
> that's really not something we can know for sure. I don't think there
> is a single safe implementation that will make everyone happy.

I think everything is, slowly, getting stricter. Generally as a result
of request smuggling vulnerabilities and the like.

> Is there a way to make this kind of thing pluggable with a few obvious
> implementations so that users can select which one makes the most
> sense for their environment? Similar to cookie-proessing, we could
> have a super-spec-compliant one which always requires CRLF line
> endings (which I guess means dropping support for HTTP 0.9), and is
> super strict about everything else.

Yes, it is possible. I'm not sure if truly pluggable or configurable
makes the most sense. I haven't looked at it too closely.

> Another implementation could work the way Tomcat currently behaves,
> being mostly spec-compliant and a little tolerant of some sloppiness.
> 
> This will allow an environment where e.g. httpd is in use to use one
> implementation while Squid, IIS, nginx, etc. can make different
> choices depending upon how the proxy will interpret things.
> 
> I realize that means more code. :( But if the proxy and origin server
> disagree about how to interpret things, Bad Things can happen. If we
> take a very strict interpretation of everything, I think we can
> actually make the origin server safe, but we may break environments
> which are relying on non-strict behavior.

So far, the biggest breakage was caused by a regression in Tomcat's
parsing of valid HTTP/0.9 requests. The approach you describe wouldn't
address that sort of issue.

We have had one additional report of new breakage with a client that
uses LFLF as the line terminator but has never been valid and it was
pure luck that Tomcat used to allow it - it was never intended to be
allowed.

My preference at this point is to concentrate on improving the unit
tests to reduce the chances of regressions as and when we do make
changes to the parsing code.

I'm not completely against making request line (and header) parsing
pluggable / configurable but relaxing parsing goes against the current
direction we have been heading in and I'd need a lot of convincing to
support such an approach.

Mark

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

Re: Request line parsing

2020-03-23 Thread Mark Thomas 
On 23/03/2020 13:28, Rémy Maucherat wrote:
> On Mon, Mar 23, 2020 at 2:01 PM Mark Thomas  > wrote:



> With all of the above in mind I propose:
> 
> - Doing nothing! I think Tomcat is striking the right balance here.
> 
> This means:
> GET /CRLF   -> processed as HTTP/0.9
> GET /LF     -> processed as HTTP/0.9
> GET / CRLF  -> processed as HTTP/1.1 and rejected as invalid
> GET / LF    -> processed as HTTP/1.1 and rejected as invalid
> 
> I want to write some tests to check this is behaving as expected but I'm
> not expecting any changes to the parsing at this point.
> 
> 
> +1, that sounds really good !

I wrote too soon :)

GET / CRLF was being parsed as malformed HTTP/0.9 so there are going to
be changes to the parsing to make "GET / LF" and "GET / CRLF" consistent
(invalid HTTP/1.1). Treating both as HTTP/1.1 I think I can see a way to
make the parsing less hacky and have the code be a little clearer.

Mark

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

[Bug 64259] New: Cannot precompile jsps with jetty-jspc-maven-plugin since 8.5.51

2020-03-23 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64259

Bug ID: 64259
   Summary: Cannot precompile jsps with jetty-jspc-maven-plugin
since 8.5.51
   Product: Tomcat 8
   Version: 8.5.53
  Hardware: PC
OS: Linux
Status: NEW
  Severity: normal
  Priority: P2
 Component: Jasper
  Assignee: dev@tomcat.apache.org
  Reporter: runningt...@googlemail.com
  Target Milestone: 

Created attachment 37117
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37117&action=edit
Very simple maven project to reproduce

Since 8.5.51 we cannot precompile jsps. We are using jspc-maven-plugin:

[WARNING] org.apache.jasper.JasperException: javax.el.ELException: Unable to
find ExpressionFactory of type [# Licensed to the Apache Software Foundation
(ASF) under one or more]

This is the stacktrace:

org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal
org.eclipse.jetty:jetty-jspc-maven-plugin:9.4.24.v20191120:jspc (jspc) on
project ...: Failure processing jsps
   at org.apache.maven.lifecycle.internal.MojoExecutor.execute
(MojoExecutor.java:215)
   at org.apache.maven.lifecycle.internal.MojoExecutor.execute
(MojoExecutor.java:156)
   at org.apache.maven.lifecycle.internal.MojoExecutor.execute
(MojoExecutor.java:148)
   at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject
(LifecycleModuleBuilder.java:117)
   at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject
(LifecycleModuleBuilder.java:81)
   at
org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build
(SingleThreadedBuilder.java:56)
   at org.apache.maven.lifecycle.internal.LifecycleStarter.execute
(LifecycleStarter.java:128)
   at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:305)
   at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192)
   at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105)
   at org.apache.maven.cli.MavenCli.execute (MavenCli.java:956)
   at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:288)
   at org.apache.maven.cli.MavenCli.main (MavenCli.java:192)
   at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
   at jdk.internal.reflect.NativeMethodAccessorImpl.invoke
(NativeMethodAccessorImpl.java:62)
   at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke
(DelegatingMethodAccessorImpl.java:43)
   at java.lang.reflect.Method.invoke (Method.java:566)
   at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced
(Launcher.java:289)
   at org.codehaus.plexus.classworlds.launcher.Launcher.launch
(Launcher.java:229)
   at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode
(Launcher.java:415)
   at org.codehaus.plexus.classworlds.launcher.Launcher.main
(Launcher.java:356)
Caused by: org.apache.maven.plugin.MojoExecutionException: Failure processing
jsps
   at org.eclipse.jetty.jspc.plugin.JspcMojo.execute (JspcMojo.java:278)
   at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo
(DefaultBuildPluginManager.java:137)
   at org.apache.maven.lifecycle.internal.MojoExecutor.execute
(MojoExecutor.java:210)
   at org.apache.maven.lifecycle.internal.MojoExecutor.execute
(MojoExecutor.java:156)
   at org.apache.maven.lifecycle.internal.MojoExecutor.execute
(MojoExecutor.java:148)
   at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject
(LifecycleModuleBuilder.java:117)
   at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject
(LifecycleModuleBuilder.java:81)
   at
org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build
(SingleThreadedBuilder.java:56)
   at org.apache.maven.lifecycle.internal.LifecycleStarter.execute
(LifecycleStarter.java:128)
   at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:305)
   at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192)
   at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105)
   at org.apache.maven.cli.MavenCli.execute (MavenCli.java:956)
   at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:288)
   at org.apache.maven.cli.MavenCli.main (MavenCli.java:192)
   at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
   at jdk.internal.reflect.NativeMethodAccessorImpl.invoke
(NativeMethodAccessorImpl.java:62)
   at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke
(DelegatingMethodAccessorImpl.java:43)
   at java.lang.reflect.Method.invoke (Method.java:566)
   at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced
(Launcher.java:289)
   at org.codehaus.plexus.classworlds.launcher.Launcher.launch
(Launcher.java:229)
   at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode
(Launcher.java:415)
   at org.codehaus.plexus.classworlds.launcher.Launcher.main
(Launcher.java:356)
Caused by: org.apache.tools.ant.BuildException: Generation completed with

Re: Remaining Tomcat 10 items

2020-03-23 Thread Rémy Maucherat 
On Mon, Mar 23, 2020 at 11:11 AM Mark Thomas  wrote:

> On 23/03/2020 09:37, Rémy Maucherat wrote:
> > Hi,
> >
> > I'm looking at the TODO list, in addition to some extra items. In order
> ...
> >
> > - Java 11.
> > I suppose Jakarta EE 9 will require Java 11, is Java 11 [going to be]
> > required for Tomcat 10 ? It could be better to do it in 10.1.
>
> No. It will be Java 8.
>
> > - Remove the use of system properties to control configuration wherever
> > possible.
> > I still don't see the point for quite a few of them. For others however,
> > using sys props was a mistake, example the facade recycling. Also, the
> > digester can now pull from system and env properties, giving full
> > flexibility. Also also, this is a handy way to do things in cloud. I
> > think we should target the ones which should make sense.
>
> For "wherever possible" read "where it makes sense to do so". My
> intention was to target those system properties that were targetted at a
> specific component and could/should be configured there.
>
> > - Reduce instances of setters and getters for the same property existing
> > on an object and its parent. This may require new objects to be exposed
> > via JMX.
> > Fixed enough ?
>
> From memory, this was primarily on the Connectors. That is certainly a
> lot better. I'd like to review things before closing this one out.
>
> > - Remove APR connector.
> > Is there still general approval for that, and is that still the plan for
> > Tomcat 10.0 ?
>
> It feels like a big step but I do think this is the right thing to do.
>
> > - Clean-up content-type header processing.
> > No opinion on that.
>
> It fixes an edge case bug. I have a plan for this. As long as it is done
> before things go stable there is no rush.
>
> > - RFC 3986 states (section 2.2) that a %nn encoded delimiter is NOT
> > equivalent to the decoded form. Provide an option not to decode
> > delimiters in %nn form.
> > Mark is working on it.
>
> I'm close to a patch for this but I got distracted with HTTP/0.9 issues.
>
> > - Refactor DefaultServlet to use Ranges in parseRanges().
> > I looked at it and I don't get it.
> > In particular the Content-Ranges header
> > (https://tools.ietf.org/html/rfc7233#section-4.2) that uses
> >
> https://github.com/apache/tomcat/blob/master/java/org/apache/tomcat/util/http/parser/ContentRange.java#L75
> > The parser here requires a '=' but that should be a space in
> > content-range (ranges uses the '=' however), the example given in the
> > spec is "Content-Range: bytes 42-1233/1234".
> > Just wondering before starting to refactor.
>
> That is the response header, not the request header. Take a look at
> sections 2.1 and 3.1
>

Right, I had forgotten that. Fun stuff.
I hope I didn't break anything with that refactoring ...

Rémy


>
> Mark
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Re: Request line parsing

2020-03-23 Thread Christopher Schultz 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Mark,

On 3/23/20 11:35, Mark Thomas wrote:
> On 23/03/2020 14:59, Christopher Schultz wrote:
>
> 
>
>> My only concern here is that request line + header-processing
>> really has to match whatever reverse proxy servers are doing as
>> well, and that's really not something we can know for sure. I
>> don't think there is a single safe implementation that will make
>> everyone happy.
>
> I think everything is, slowly, getting stricter. Generally as a
> result of request smuggling vulnerabilities and the like.
>
>> Is there a way to make this kind of thing pluggable with a few
>> obvious implementations so that users can select which one makes
>> the most sense for their environment? Similar to
>> cookie-proessing, we could have a super-spec-compliant one which
>> always requires CRLF line endings (which I guess means dropping
>> support for HTTP 0.9), and is super strict about everything
>> else.
>
> Yes, it is possible. I'm not sure if truly pluggable or
> configurable makes the most sense. I haven't looked at it too
> closely.
>
>> Another implementation could work the way Tomcat currently
>> behaves, being mostly spec-compliant and a little tolerant of
>> some sloppiness.
>>
>> This will allow an environment where e.g. httpd is in use to use
>> one implementation while Squid, IIS, nginx, etc. can make
>> different choices depending upon how the proxy will interpret
>> things.
>>
>> I realize that means more code. :( But if the proxy and origin
>> server disagree about how to interpret things, Bad Things can
>> happen. If we take a very strict interpretation of everything, I
>> think we can actually make the origin server safe, but we may
>> break environments which are relying on non-strict behavior.
>
> So far, the biggest breakage was caused by a regression in
> Tomcat's parsing of valid HTTP/0.9 requests. The approach you
> describe wouldn't address that sort of issue.
>
> We have had one additional report of new breakage with a client
> that uses LFLF as the line terminator but has never been valid and
> it was pure luck that Tomcat used to allow it - it was never
> intended to be allowed.
>
> My preference at this point is to concentrate on improving the
> unit tests to reduce the chances of regressions as and when we do
> make changes to the parsing code.
>
> I'm not completely against making request line (and header)
> parsing pluggable / configurable but relaxing parsing goes against
> the current direction we have been heading in and I'd need a lot of
> convincing to support such an approach.

Sounds good. I entirely missed your actual proposal, which was below
your signature and after your references:

On 3/23/20 09:01, Mark Thomas wrote:
> With all of the above in mind I propose:
>
> - Doing nothing! I think Tomcat is striking the right balance
> here.
>
> This means: GET /CRLF   -> processed as HTTP/0.9 GET /LF ->
> processed as HTTP/0.9 GET / CRLF  -> processed as HTTP/1.1 and
> rejected as invalid GET / LF-> processed as HTTP/1.1 and
> rejected as invalid
>
> I want to write some tests to check this is behaving as expected
> but I'm not expecting any changes to the parsing at this point.
I think I like the above very much.

- -chris
-BEGIN PGP SIGNATURE-
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl548uMACgkQHPApP6U8
pFgZRQ/+KCX+EgM6+5FLDacDw2dy0mFSdCkjnWKcq2WbgwZe6GbNeGra0Egme9GF
CsJmVwrHnv34y+VKuvugoKVxDMyvsrkIrLyYPyW+VJg+iualK74BEI/28lvMGt2M
4RuPofhNQNmNxbWP1cvcWGb1CEinVo97AU3lHjRHRCtWWL5jQhiO9te9GWk2Q9z9
EmLJZo/tS+PSYLx73ewFEN0BxH7exQAu3MsYNxtq/be7pEDj4kJF5tXY6vnSXbmX
upZ14tBpw/loy6aC/Ay56Nx76q/t0+J4ZNsYgsNf3OeowCYnWFw8orbM5rCVFuY2
CshC40F+rhc1IZfcVd4F34SqRGs3W47eA4/CHnKBHBMA8R6Oj76gJkJvn3izRLYH
y5L8kHc3umXXDYQkDYgytGzGDaaXx0+OKaZHLywV6rXw30DvANh6xhsW+7GKqqI6
8FckjrQeCrBzBg+tSz+ObeHJxZvhpD5CYg5vjH8YxXKL6hi65k+IaaOgydmjq2SQ
lnoMIfdmydxRYnmhmzfCcJdgxkctvg3AGyidG2zEWE8EYbAfzixb9PVDZEhb4lMk
wBE5wj2IUmgsmFV/MdUfQjZNFn8dEoZbjebrrIyUlsLJng7AIObXss3myp6DZJXt
OWSjnMGVcsvXzbpfs/nw9kFnkdEMFbzDnVVGBDf93KygLQqEgCM=
=zQMS
-END PGP SIGNATURE-

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

Re: Request line parsing

2020-03-23 Thread Mark Thomas 
On 23/03/2020 17:33, Christopher Schultz wrote:
> On 3/23/20 11:35, Mark Thomas wrote:



> Sounds good. I entirely missed your actual proposal, which was below
> your signature and after your references:

Sorry about that. I was editing and re-organising and got distracted.

Mark

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

[Bug 63691] Add a no-op JarScanner

2020-03-23 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=63691

--- Comment #11 from Gustavo Stachera  ---
(In reply to Joshua Lipstone from comment #8)
> Can you please either undo this or change it so that the Jars are only
> scanned if they match the inclusion filter.
> As of 9.0.30, if you wanted to set the logic so that it only scans a short
> list of jars, you could do:
> jarsToSkip=*
> jarsToScan=jar1.jar,jar2.jar
> As of 9.0.31, this now causes cascading startup failures.

I've implemented a workaround using de wildcard "?" in the beginning, followed
by "*" and it works:

tomcat.util.scan.StandardJarScanFilter.jarsToSkip=?*

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: Request line parsing

2020-03-23 Thread Filip Hanik 
+1

Thorough and clear write up

On Mon, Mar 23, 2020 at 06:01 Mark Thomas  wrote:

> Hi,
>
> I am currently looking at the request line parsing. I'll try and set out
> each issue in turn.
>
> End of line parsing
> ===
>
> Prior to the recent changes, Tomcat allowed CRLF or LF to mark the end
> of a line. The unwanted side effect was that CR could appear in the
> header value. This caused problems and was tightened up to only allow
> CRLF as a line terminator.
>
> Currently Tomcat requires CRLF everywhere apart from the end of the
> request line for a HTTP 0.9 request where it also allows LF.
>
> This requirement to accept just LF as a line terminator first emerged in
> the W3C spec [1]. RFC 1945 [2] and RFC 2616 [3] retained this as a
> recommendation for all line terminators, RFC 7230 [4] no longer includes
> this recommendation.
>
> RFC 7230 also removes the expectation that a server that supports
> HTTP/1.1 will support HTTP 0.9.
>
> Arguably the current spec for HTTP/0.9 is [3].
>
> The Servlet spec references RFC 7230 and RFC 1945 so arguably HTTP/0.9
> support is expected.
>
>
> SP vs whitespace
> 
>
> Tomcat currently accepts any combination of SP and HTAB where RFC 7230
> calls for a single SP. This stems from a recommendation in RFC 2616
> which is no longer present in RFC 7230.
>
>
> I think we have three options.
>
> 1. No changes.
>CRLF is required everywhere apart from HTTP/0.9 where LF is also
>accepted.
>Any combination of SP/HTAB is accepted where SP is required.
>
> 2. Tighten up as per RFC 7230
>a) Require CRLF for all line endings
>b) Require SP where specified
>c) Drop HTTP/0.9 support
>
> 3. Relax the recent changes to allow CRLF or LF as a line terminator
>everywhere without allowing CR to appear in a request header.
>
> I think we should follow 1) for Tomcat 7, 8 & 9.
>
> I'm leaning towards 1 for 10.0.x as well with a view to discussing 2 in
> the Servlet project. i.e. explicitly dropping HTTP 0.9 support and the
> "Tolerant applications" requirements of RFC 1945 for Jakarta EE 10
> (Tomcat 10.1.x).
>
> In short this means largely do nothing apart from may be adding a few
> more tests to explicitly check behaviour for various edge cases.
>
> I'll note that the regressions reported with the recent change to
> requiring CRLF as a line terminator caused issues with valid HTTP/0.9
> requests but that this should now be resolved.
>
> We have had one user issue reported where a custom client was using LFLF
> as a line terminator and requests are now being rejected. Given that was
> never valid, I'm OK with that.
>
> Thoughts?
>
> Mark
>
>
>
> [1] https://www.w3.org/Protocols/HTTP/AsImplemented.html
> [2] https://tools.ietf.org/html/rfc1945
> [3] https://tools.ietf.org/html/rfc2616
> [4] https://tools.ietf.org/html/rfc7230
>
>
>
>
>
> With all of the above in mind I propose:
>
> - Doing nothing! I think Tomcat is striking the right balance here.
>
> This means:
> GET /CRLF   -> processed as HTTP/0.9
> GET /LF -> processed as HTTP/0.9
> GET / CRLF  -> processed as HTTP/1.1 and rejected as invalid
> GET / LF-> processed as HTTP/1.1 and rejected as invalid
>
> I want to write some tests to check this is behaving as expected but I'm
> not expecting any changes to the parsing at this point.
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>