svn commit: r1600833 - /tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java

[markt]

Author: markt
Date: Fri Jun  6 09:19:17 2014
New Revision: 1600833

URL: http://svn.apache.org/r1600833
Log:
Don't assume that tests use the StandardManger

Modified:
tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java

Modified: tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java?rev=1600833&r1=1600832&r2=1600833&view=diff
==
--- tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java (original)
+++ tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java Fri Jun  
6 09:19:17 2014
@@ -42,11 +42,13 @@ import org.apache.catalina.Container;
 import org.apache.catalina.Context;
 import org.apache.catalina.LifecycleException;
 import org.apache.catalina.LifecycleState;
+import org.apache.catalina.Manager;
 import org.apache.catalina.Server;
 import org.apache.catalina.Service;
 import org.apache.catalina.connector.Connector;
 import org.apache.catalina.core.AprLifecycleListener;
 import org.apache.catalina.core.StandardServer;
+import org.apache.catalina.session.ManagerBase;
 import org.apache.catalina.session.StandardManager;
 import org.apache.catalina.valves.AccessLogValve;
 import org.apache.coyote.http11.Http11NioProtocol;
@@ -393,14 +395,15 @@ public abstract class TomcatBaseTest ext
 Container e = service.getContainer();
 for (Container h : e.findChildren()) {
 for (Container c : h.findChildren()) {
-StandardManager m =
-(StandardManager) ((Context) c).getManager();
+Manager m = ((Context) c).getManager();
 if (m == null) {
 m = new StandardManager();
-m.setSecureRandomClass(
-
"org.apache.catalina.startup.FastNonSecureRandom");
 ((Context) c).setManager(m);
 }
+if (m instanceof ManagerBase) {
+((ManagerBase) m).setSecureRandomClass(
+
"org.apache.catalina.startup.FastNonSecureRandom");
+}
 }
 }
 }



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

buildbot retry in ASF Buildbot on tomcat-trunk

[buildbot]

 on builder tomcat-trunk while building ASF Buildbot.
Full details are available at:
 http://ci.apache.org/builders/tomcat-trunk/builds/151

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: bb-vm_ubuntu

Build Reason: scheduler
Build Source Stamp: [branch tomcat/trunk] 1600833
Blamelist: markt

BUILD FAILED: retry exception slave lost

sincerely,
 -The Buildbot




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

svn commit: r1600839 - in /tomcat/trunk: java/org/apache/catalina/ha/session/DeltaSession.java test/org/apache/catalina/session/TestStandardSession.java webapps/docs/changelog.xml

[markt]

Author: markt
Date: Fri Jun  6 10:25:49 2014
New Revision: 1600839

URL: http://svn.apache.org/r1600839
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56578
Correct regression in the fix for 56339 that prevented sessions from expiring 
when using clustering.
The effect of removing expiring = true; from the DeltaSession is that if 
parallel calls are made to expire() then there is a slightly greater chance of 
some of those calls blocking while the first completes. This is because 
expiring is now set to true a little later.

Added:
tomcat/trunk/test/org/apache/catalina/session/TestStandardSession.java   
(with props)
Modified:
tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java?rev=1600839&r1=1600838&r2=1600839&view=diff
==
--- tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java Fri Jun  
6 10:25:49 2014
@@ -446,10 +446,6 @@ public class DeltaSession extends Standa
 if (manager == null)
 return;
 
-// Mark this session as "being expired". The flag will be unset in
-// the call to super.expire(notify)
-expiring = true;
-
 String expiredId = getIdInternal();
 
 if(notifyCluster && expiredId != null &&

Added: tomcat/trunk/test/org/apache/catalina/session/TestStandardSession.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/session/TestStandardSession.java?rev=1600839&view=auto
==
--- tomcat/trunk/test/org/apache/catalina/session/TestStandardSession.java 
(added)
+++ tomcat/trunk/test/org/apache/catalina/session/TestStandardSession.java Fri 
Jun  6 10:25:49 2014
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.session;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.ha.tcp.SimpleTcpCluster;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
+
+public class TestStandardSession extends TomcatBaseTest {
+
+/**
+ * Test session.invalidate() in a clustered environment.
+ */
+@Test
+public void testBug56578a() throws Exception {
+doTestInvalidate(true);
+}
+
+@Test
+public void testBug56578b() throws Exception {
+doTestInvalidate(false);
+}
+
+private void doTestInvalidate(boolean useClustering) throws Exception {
+// Setup Tomcat instance
+Tomcat tomcat = getTomcatInstance();
+
+// Must have a real docBase - just use temp
+Context ctx = tomcat.addContext("", 
System.getProperty("java.io.tmpdir"));
+
+Tomcat.addServlet(ctx, "bug56578", new Bug56578Servlet());
+ctx.addServletMapping("/bug56578", "bug56578");
+
+if (useClustering) {
+tomcat.getEngine().setCluster(new SimpleTcpCluster());
+ctx.setDistributable(true);
+ctx.setManager(ctx.getCluster().createManager(""));
+}
+tomcat.start();
+
+ByteChunk res = getUrl("http://localhost:"; + getPort() + "/bug56578");
+Assert.assertEquals("PASS", res.toString());
+}
+
+private static class Bug56578Servlet extends HttpServlet {
+
+private static final long serialVersionUID = 1L;
+
+@Override
+protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+throws ServletException, IOException {
+   

svn commit: r1600840 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/ha/session/DeltaSession.java test/org/apache/catalina/session/TestStandardSession.java webapps/docs/changelog.xml

[markt]

Author: markt
Date: Fri Jun  6 10:33:15 2014
New Revision: 1600840

URL: http://svn.apache.org/r1600840
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56578
Correct regression in the fix for 56339 that prevented sessions from expiring 
when using clustering.
The effect of removing expiring = true; from the DeltaSession is that if 
parallel calls are made to expire() then there is a slightly greater chance of 
some of those calls blocking while the first completes. This is because 
expiring is now set to true a little later.

Added:

tomcat/tc7.0.x/trunk/test/org/apache/catalina/session/TestStandardSession.java
  - copied unchanged from r1600839, 
tomcat/trunk/test/org/apache/catalina/session/TestStandardSession.java
Modified:
tomcat/tc7.0.x/trunk/   (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc7.0.x/trunk/
--
  Merged /tomcat/trunk:r1600839

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/DeltaSession.java?rev=1600840&r1=1600839&r2=1600840&view=diff
==
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/DeltaSession.java 
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/DeltaSession.java 
Fri Jun  6 10:33:15 2014
@@ -459,10 +459,6 @@ public class DeltaSession extends Standa
 if (manager == null)
 return;
 
-// Mark this session as "being expired". The flag will be unset in
-// the call to super.expire(notify)
-expiring = true;
-
 String expiredId = getIdInternal();
 
 if(notifyCluster && expiredId != null &&

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1600840&r1=1600839&r2=1600840&view=diff
==
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Fri Jun  6 10:33:15 2014
@@ -81,6 +81,10 @@
 56588: Update deprecation of 
Context.addApplicationListener()
 methods according to changes in Tomcat 8. (kkolinko)
   
+  
+56578: Correct regression in the fix for 56339
+that prevented sessions from expiring when using clustering. (markt)
+  
 
   
   



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

[Bug 56578] session.invalidate does not work on cluster enabled webapps

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=56578

Mark Thomas  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #8 from Mark Thomas  ---
This has been fixed in 8.0.x for 8.0.9 onwards and in 7.0.x for 7.0.55 onwards.

-- 
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

The Early Access build for JDK 8u20 b16 is available on java.net

[Rory O'Donnell Oracle, Dublin Ireland]

Hi Mark,

The backport fix for JDK-8041481 
 is in JDK 8u20 b17 as 
JDK-8039751 , 
available for download  .

Can you test and report results ?

Rgds,Rory

--
Rgds,Rory O'Donnell
Quality Engineering Manager
Oracle EMEA , Dublin, Ireland

Re: Error handling

[Konstantin Kolinko]

2014-05-29 16:31 GMT+04:00 Mark Thomas :
> You've probably noticed I've been poking around the ErrorReportValve,
> the Response objects and other related code. I've been looking into an
> issue that one of the groups at $work has raised around Tomcat's error
> handling and I've been cleaning things up as I notice them to stop me
> getting distracted while I try and concentrate on the larger problem.
>
> I've also spotted a few places where I think we are duplicating
> functionality unnecessarily. I'm trying to clean this up as well. Partly
> because it is more efficient and partly because the code is easier to
> follow without it.
>
> So, that larger problem...
>
> It is to do with how uncaught exceptions are handled.
> ...
>

Reviewing r1600449 and followups.

// ErrorReportValve.java
[[[
if (response.isCommitted()) {
if (response.isErrorAfterCommit()) {
// Flush any data that is still to be written to the client
response.flushBuffer();
// Close immediately to signal to the client that something went
// wrong
response.getCoyoteResponse().action(ActionCode.CLOSE_NOW, null);
}
return;
}
]]]

1. If original error was due to I/O failure, then I expect
"response.flushBuffer()" call to fail with an IOException.

In ErrorReportValve#report() any IOException is silently swallowed. So
I think we can swallow it here.


// ErrorReportValve.java
[[[
if (throwable != null) {
// Make sure that the necessary methods have been called on the
// response. (It is possible a component may just have set the
// Throwable. Tomcat won't do that but other components might.)
// These are safe to call at this point as we know that the response
// has not been committed.
response.reset();
response.sendError(
HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}

// One way or another, response.sendError() will have been called before
// execution reaches this point and suspended the response. Need to
// reverse that so this valve can write to the response.
response.setSuspended(false);

try {
report(request, response, throwable);
} catch (Throwable tt) {
ExceptionUtils.handleThrowable(tt);
}
]]]

I know that the above calls were essentially present before these
changes. Looking at them I have the following thoughts:

2. Looking at "if (throwable != null)".
The error might have already been handled either by custom error page
(StandardHostValve.custom(...)) or if there are several
ErrorReportValve valves in the chain.

In both cases the RequestDispatcher.ERROR_EXCEPTION attribute is not cleared.

The StandardHostValve saves us from this by always calling
response.flushBuffer();
just after invoking its custom() method. Thus "if
response.isCommitted()" branch is triggered and the error report valve
invocation returns early.

ErrorReportValve.report() can be updated to call flushBuffer() as
well. Otherwise I think if there are several ErrorReportValve valves
the "if throwable != null" branch will reset the response and it will
be generated a-new.

Though if there is no other good reason to flush, then it looks like a
hack (in StandardHostValve as well). (A known drawback of
flushBuffer() is that it causes chunked encoding with HTTP/1.1, even
if the whole response fits in a buffer).

I wonder if those flushBuffer() calls can be removed.


3. Some Tomcat components set RequestDispatcher.ERROR_EXCEPTION
attribute without setting any other error flags.

E.g. o.a.c.connector.Request#notifyAttributeAssigned),
#notifyAttributeRemoved().

I think this is the only use case where "if (throwable != null)"
branch and response.reset() call there are justified.

If response have been already committed the "if
response.isCommitted()" branch will be triggered, but not "if
response.isErrorAfterCommit()", as error flag has not been set.

So I interpret this use case as "Some error was detected, but it is
not fatal. If response have not been committed then report it. If it
has been committed, ignore it".


For 3. I think maybe do the following:
Replace "if (throwable != null)"
with "if (throwable != null && !response.isError())"

If the above replacement is done then I think that maybe those
flushBuffer() calls in StandardHostValve mentioned in "2." can be
removed.


Best regards,
Konstantin Kolinko

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

buildbot failure in ASF Buildbot on tomcat-7-trunk

[buildbot]

The Buildbot has detected a new failure on builder tomcat-7-trunk while 
building ASF Buildbot.
Full details are available at:
 http://ci.apache.org/builders/tomcat-7-trunk/builds/97

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: bb-vm_ubuntu

Build Reason: scheduler
Build Source Stamp: [branch tomcat/tc7.0.x/trunk] 1600840
Blamelist: markt

BUILD FAILED: failed compile_1

sincerely,
 -The Buildbot




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

svn commit: r1600846 - /tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

[kkolinko]

Author: kkolinko
Date: Fri Jun  6 11:38:57 2014
New Revision: 1600846

URL: http://svn.apache.org/r1600846
Log:
Fix ordering of changelog entries according to bug number.

Modified:
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1600846&r1=1600845&r2=1600846&view=diff
==
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Fri Jun  6 11:38:57 2014
@@ -77,14 +77,14 @@
 may otherwise be triggered by a web application which in turn would
 trigger an exception when running under a security manager. (kkolinko)
   
-  
-56588: Update deprecation of 
Context.addApplicationListener()
-methods according to changes in Tomcat 8. (kkolinko)
-  
   
 56578: Correct regression in the fix for 56339
 that prevented sessions from expiring when using clustering. (markt)
   
+  
+56588: Update deprecation of 
Context.addApplicationListener()
+methods according to changes in Tomcat 8. (kkolinko)
+  
 
   
   



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

Re: buildbot failure in ASF Buildbot on tomcat-7-trunk

[Konstantin Kolinko]

2014-06-06 15:37 GMT+04:00  :
> The Buildbot has detected a new failure on builder tomcat-7-trunk while 
> building ASF Buildbot.
> Full details are available at:
>  http://ci.apache.org/builders/tomcat-7-trunk/builds/97
>
> Buildbot URL: http://ci.apache.org/
>
> Buildslave for this Build: bb-vm_ubuntu
>
> Build Reason: scheduler
> Build Source Stamp: [branch tomcat/tc7.0.x/trunk] 1600840
> Blamelist: markt
>
> BUILD FAILED: failed compile_1

Test org.apache.catalina.session.TestStandardSession FAILED
BIO, NIO.

Testsuite: org.apache.catalina.session.TestStandardSession
Tests run: 2, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 3.173 sec
-  ---

Testcase: testBug56578a took 0.743 sec
Caused an ERROR
org.apache.catalina.ha.session.DeltaManager cannot be cast to
org.apache.catalina.session.StandardManager
java.lang.ClassCastException:
org.apache.catalina.ha.session.DeltaManager cannot be cast to
org.apache.catalina.session.StandardManager
at 
org.apache.catalina.startup.TomcatBaseTest$TomcatWithFastSessionIDs.start(TomcatBaseTest.java:375)
at 
org.apache.catalina.session.TestStandardSession.doTestInvalidate(TestStandardSession.java:67)
at 
org.apache.catalina.session.TestStandardSession.testBug56578a(TestStandardSession.java:44)

Testcase: testBug56578b took 2.391 sec

Best regards,
Konstantin Kolinko

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

svn commit: r1600858 - /tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java

[markt]

Author: markt
Date: Fri Jun  6 12:02:38 2014
New Revision: 1600858

URL: http://svn.apache.org/r1600858
Log:
Don't assume that tests use the StandardManger

Modified:
tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java

Modified: 
tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java?rev=1600858&r1=1600857&r2=1600858&view=diff
==
--- tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java 
(original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java 
Fri Jun  6 12:02:38 2014
@@ -41,11 +41,13 @@ import org.junit.Before;
 import org.apache.catalina.Container;
 import org.apache.catalina.LifecycleException;
 import org.apache.catalina.LifecycleState;
+import org.apache.catalina.Manager;
 import org.apache.catalina.Server;
 import org.apache.catalina.Service;
 import org.apache.catalina.connector.Connector;
 import org.apache.catalina.core.AprLifecycleListener;
 import org.apache.catalina.core.StandardServer;
+import org.apache.catalina.session.ManagerBase;
 import org.apache.catalina.session.StandardManager;
 import org.apache.catalina.valves.AccessLogValve;
 import org.apache.tomcat.util.buf.ByteChunk;
@@ -372,13 +374,15 @@ public abstract class TomcatBaseTest ext
 Container e = service.getContainer();
 for (Container h : e.findChildren()) {
 for (Container c : h.findChildren()) {
-StandardManager m = (StandardManager) c.getManager();
+Manager m = c.getManager();
 if (m == null) {
 m = new StandardManager();
-m.setSecureRandomClass(
-
"org.apache.catalina.startup.FastNonSecureRandom");
 c.setManager(m);
 }
+if (m instanceof ManagerBase) {
+((ManagerBase) m).setSecureRandomClass(
+
"org.apache.catalina.startup.FastNonSecureRandom");
+}
 }
 }
 }



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

svn commit: r1600862 - /tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java

[kkolinko]

Author: kkolinko
Date: Fri Jun  6 12:10:18 2014
New Revision: 1600862

URL: http://svn.apache.org/r1600862
Log:
Verify that response is flushed when servlet fails with an unexpected exception.

Modified:
tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java

Modified: 
tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java?rev=1600862&r1=1600861&r2=1600862&view=diff
==
--- tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java 
(original)
+++ tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java 
Fri Jun  6 12:10:18 2014
@@ -82,6 +82,8 @@ public class TestAbstractHttp11Processor
 assertTrue(client.isResponse200());
 // There should not be an end chunk
 assertFalse(client.getResponseBody().endsWith("0"));
+// The last portion of text should be there
+assertTrue(client.getResponseBody().endsWith("line03"));
 }
 
 private static class ResponseWithErrorServlet extends HttpServlet {



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

[Bug 56580] el-api.jar memory leak

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=56580

Mark Thomas  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |WONTFIX

--- Comment #7 from Mark Thomas  ---
Hmm. The specification is completely silent on this. There is the odd hint that
caching may take place within the EL implementation but no details are provided
as to what might be cached or how and, importantly, no API is provided to clear
any cache that may exist.

The Glassfish solution was to use SoftReferences for the cache entries. That
only sort of fixes the problem in that the web app class loader could remain in
memory until long after the web application is reloaded. We have seen cases in
the past where this behavior causes problems for some libraries (and is why we
added closeMethod support to resources).

If the cache was static then we could add a method to clear the cache for a
given class loader and call that via reflection when the web application stops.
However, making the cache static is going to have all sorts of side-effects -
some of which may well be unpleasant. The most obvious of these is a guaranteed
memory leak unless you call the clear cache method which most clients won't do.

The other pure Tomcat solutions are don't cache or to place the Mojarra JARs in
the web application rather than $CATALINA_BASE/lib.

Mojarra could use per web application instances of BeanELResolver rather than a
single static instance. Mojarra should also have the necessary hooks to remove
those instances when the web application stops (e.g. ServletContextListener).

In short, I don't see a way to fix this safely in Tomcat. I think it could be
fixed in Mojarra but I don't know that code base so I might be completely
wrong. There are a couple of work-arounds: disable the cache, move the Mojarra
JARs to WEB=INF/lib.

I agree that there is a problem here and I'd like to fix it in Tomcat if I
could but I don't see a way to do that. Given that there are workarounds
available I am going to resolve this as WONTFIX.

-- 
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

Time for 8.0.9

[Mark Thomas]

As I have said before, I'd like to get back to my original plane for
monthly releases of 8.0.x. With that in mind - and unless some new bugs
appear in the next few hours - I plan to tag 8.0.9 later today.

Mark

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

[Bug 56581] When an error occurs on a long JSP page, do not loose last chunk of printed text

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=56581

--- Comment #3 from Konstantin Kolinko  ---
Looking into generated test_jsp.java,  I see that _jspService method ends with
the following:

[[[
} catch (java.lang.Throwable t) {
  if (!(t instanceof javax.servlet.jsp.SkipPageException)){
out = _jspx_out;
if (out != null && out.getBufferSize() != 0)
  try { out.clearBuffer(); } catch (java.io.IOException e) {}
if (_jspx_page_context != null)
_jspx_page_context.handlePageException(t);
else throw new ServletException(t);
  }
} finally {
]]]

So there is a call to JspWriterImpl.clearBuffer() that clears its buffer.

I can think of two ways to fix:
a) If I had access to JspWriterImpl internals then,

if (out.flushed)
 out.flushBuffer();
else
 out.clearBuffer();

The motivation is that if we cannot undo, then do not lose what we already have
generated.

I think this way is wrong. The underlying stream can have a larger buffer.
While out.flushed is true, the underlying stream may not have been flushed yet.

b) 
if (response.isCommitted()) {
 out.flush();
} else {
 out.clearBuffer();
}

Although "b)" is a bit more invasive, I think it is a way to go. It is also
good that it can be implemented using public APIs.

-- 
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

buildbot failure in ASF Buildbot on tomcat-trunk

[buildbot]

The Buildbot has detected a new failure on builder tomcat-trunk while building 
ASF Buildbot.
Full details are available at:
 http://ci.apache.org/builders/tomcat-trunk/builds/153

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: bb-vm_ubuntu

Build Reason: scheduler
Build Source Stamp: [branch tomcat/trunk] 1600833
Blamelist: markt

BUILD FAILED: failed compile_1

sincerely,
 -The Buildbot




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

svn commit: r1600899 - in /tomcat/trunk: java/org/apache/jasper/compiler/Generator.java webapps/docs/changelog.xml

[kkolinko]

Author: kkolinko
Date: Fri Jun  6 14:36:36 2014
New Revision: 1600899

URL: http://svn.apache.org/r1600899
Log:
Fix http://issues.apache.org/bugzilla/show_bug.cgi?id=56581
If an error on a JSP page occurs when response have already been committed,
do not clear the buffer of JspWriter, but flush it.

Modified:
tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Generator.java?rev=1600899&r1=1600898&r2=1600899&view=diff
==
--- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Fri Jun  6 
14:36:36 2014
@@ -3393,7 +3393,19 @@ class Generator {
 out.printil("out = _jspx_out;");
 out.printil("if (out != null && out.getBufferSize() != 0)");
 out.pushIndent();
-out.printil("try { out.clearBuffer(); } catch (java.io.IOException e) 
{}");
+out.printil("try {");
+out.pushIndent();
+out.printil("if (response.isCommitted()) {");
+out.pushIndent();
+out.printil("out.flush();");
+out.popIndent();
+out.printil("} else {");
+out.pushIndent();
+out.printil("out.clearBuffer();");
+out.popIndent();
+out.printil("}");
+out.popIndent();
+out.printil("} catch (java.io.IOException e) {}");
 out.popIndent();
 out.printil("if (_jspx_page_context != null) 
_jspx_page_context.handlePageException(t);");
 out.printil("else throw new ServletException(t);");

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1600899&r1=1600898&r2=1600899&view=diff
==
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Jun  6 14:36:36 2014
@@ -189,6 +189,11 @@
 56568: Allow any HTTP method when a JSP is being used as an
 error page. (markt)
   
+  
+56581: If an error on a JSP page occurs when response have
+already been committed, do not clear the buffer of JspWriter, but flush
+it. Thus it will become more clear where the error occurred. (kkolinko)
+  
 
   
   



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

[Bug 56581] When an error occurs on a long JSP page, do not loose last chunk of printed text

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=56581

Konstantin Kolinko  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #4 from Konstantin Kolinko  ---
Committed in r1600899 and will be in 8.0.9.

By the way, the try/catch{IOException} on those lines originates from r451231.

-- 
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

[Bug 54618] Add filter implementing HTTP Strict Transport Security (HSTS) [PATCH]

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=54618

Christopher Schultz  changed:

   What|Removed |Added

  Attachment #3|application/octet-stream|text/plain
  mime type||

-- 
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

[Bug 54618] Add filter implementing HTTP Strict Transport Security (HSTS) [PATCH]

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=54618

--- Comment #9 from Christopher Schultz  ---
(In reply to Steve Sether from comment #8)
> I think this is an important feature for Tomcat to support out of the box. 

Then vote for it: there are currently 5 votes from a single person for this
issue. More votes = more attention.

> So this is incredibly trivial to do in Apache since adding headers is very,
> very easy.  It's far harder to do this on Tomcat since it requires code
> modifications.  Why can't Tomcat have a similar feature?

Adding a Filter (assuming it's already been written/compiled) only requires
configuration, just like Apache httpd. If you don't have mod_headers, it
"requires a code change" just as this does.

The Filter is attached to this issue. Feel free to download it and use it. It
just hasn't made it into Tomcat's distribution yet.

> IMO the solution should be broader than just this one header, and should be
> a simple config option that an admin can add or subtract rather than having
> to implement this on every web application.

The Filter can be added to conf/web.xml and will apply to all web applications
hosted by the container. I'm not sure in what order it will be applied, though.
My wild guess without trying is that everything in conf/web.xml will be applied
first, then all the filters defined in the application's WEB-INF/web.xml.

> I think it's vitally important that the admin should be able to control
> this, since the security feature it implements crosses multiple applications
> on a server, not just one.  That's something a good administrator can
> implement quickly, and would be far harder and more error prone to add at
> the application level.

Admins have the ability to modify conf/web.xml.

-- 
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

[Bug 54618] Add filter implementing HTTP Strict Transport Security (HSTS) [PATCH]

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=54618

--- Comment #10 from Christopher Schultz  ---
I think there is a bug in the Filter implementation provided by Jens:

The filter calls chain.doFilter() and then adds the header afterward. This
isn't going to work for any request that generates output that exceeds the
buffer size, or that explicitly calls getOutputStream().flush (or
getWriter().flush).

Is anyone using this Filter implementation anywhere, or was it a
proof-of-concept?

-- 
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

[Bug 54618] Add filter implementing HTTP Strict Transport Security (HSTS) [PATCH]

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=54618

--- Comment #11 from Konstantin Kolinko  ---
(In reply to Christopher Schultz from comment #9)
> 
> The Filter can be added to conf/web.xml and will apply to all web
> applications hosted by the container. I'm not sure in what order it will be
> applied, though. My wild guess without trying is that everything in
> conf/web.xml will be applied first, then all the filters defined in the
> application's WEB-INF/web.xml.

To clarify: the order is the opposite in Tomcat 7 onwards. The WEB-IND/web.xml
filters are first, the ones in conf/web.xml are second. This is documented in
Migration Guide.

http://tomcat.apache.org/migration-7.html#Processing_of_conf/web.xml_file

-- 
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

[Bug 54618] Add filter implementing HTTP Strict Transport Security (HSTS) [PATCH]

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=54618

--- Comment #12 from Konstantin Kolinko  ---
(In reply to Steve Sether from comment #8)
> 
> Furthermore though, headers like this should be insanely easy to just add to
> all the headers of a domain hosted on a machine.  Apache solves this very
> easily with a single configuration line:
> 
> Header add Strict-Transport-Security "max-age=15768000"

If it is that simple, maybe just use the well-known UrlRewriteFilter, as linked
here:
http://wiki.apache.org/tomcat/AddOns#UrlRewrite

A  rule will set a response header.

-- 
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

[Bug 55975] Inconsistent escaping applied to V0 cookie values

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=55975

Konstantin Kolinko  changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|FIXED   |---

--- Comment #2 from Konstantin Kolinko  ---
In org.apache.tomcat.util.http.TestSetCookieSupportSeparatorsAllowed there are
3 tests that are marked with  @Ignore("bug 55975")

If I remove the @Ignore on them, 1 test passes, 2 tests fail.

One that passes:

Testcase: v1ValueContainsNonV0Separator took 0 sec

Two that fail:

Testcase: v1ValueContainsBackslashAndQuote took 0,008 sec
FAILED
expected: but was:

Testcase: v1ValueContainsBackslash took 0 sec
FAILED
expected: but was:

-- 
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

svn commit: r1600948 - /tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java

[kkolinko]

Author: kkolinko
Date: Fri Jun  6 16:09:16 2014
New Revision: 1600948

URL: http://svn.apache.org/r1600948
Log:
https://issues.apache.org/bugzilla/show_bug.cgi?id=56568
Fix missing "return" statement for JSP method name check.

Modified:
tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java

Modified: tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java?rev=1600948&r1=1600947&r2=1600948&view=diff
==
--- tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java (original)
+++ tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java Fri Jun  6 
16:09:16 2014
@@ -292,6 +292,7 @@ public class JspServlet extends HttpServ
 // against verb tampering
 response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED,
 Localizer.getMessage("jsp.error.servlet.invalid.method"));
+return;
 }
 
 //jspFile may be configured as an init-param for this servlet instance



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

[Bug 56600] New: Missing "return" statements in WebdavServlet.doPropfind()

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=56600

Bug ID: 56600
   Summary: Missing "return" statements in
WebdavServlet.doPropfind()
   Product: Tomcat 8
   Version: 8.0.8
  Hardware: PC
Status: NEW
  Severity: minor
  Priority: P2
 Component: Catalina
  Assignee: dev@tomcat.apache.org
  Reporter: knst.koli...@gmail.com

In WebdavServlet.doPropfind() method that handles PROPFIND requests there are
two resp.sendError() method calls that are not followed by "return;".

It should not cause a visible error, but just some wasted work that generates
response that is never sent to the 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

svn commit: r1600950 - in /tomcat/trunk: java/org/apache/tomcat/util/net/Nio2Endpoint.java webapps/docs/changelog.xml

[remm]

Author: remm
Date: Fri Jun  6 16:14:44 2014
New Revision: 1600950

URL: http://svn.apache.org/r1600950
Log:
Try removing the beta tag from the NIO2 connector to get more feedback, after 
the latest round of fixes. In practice since it is not used by default, it 
doesn't change much.

Modified:
tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1600950&r1=1600949&r2=1600950&view=diff
==
--- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Fri Jun  6 
16:14:44 2014
@@ -353,9 +353,6 @@ public class Nio2Endpoint extends Abstra
 running = true;
 paused = false;
 
-// FIXME: remove when more stable
-log.warn("The NIO2 connector is currently BETA and should not be 
used in production");
-
 // Create worker collection
 if ( getExecutor() == null ) {
 createExecutor();

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1600950&r1=1600949&r2=1600950&view=diff
==
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Jun  6 16:14:44 2014
@@ -158,6 +158,9 @@
 make it easier for the client to differentiate between a complete
 response and one that failed part way though. (markt)
   
+  
+Remove the beta tag from the NIO2 connectors. (remm)
+  
 
   
   



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

Re: svn commit: r1600950 - in /tomcat/trunk: java/org/apache/tomcat/util/net/Nio2Endpoint.java webapps/docs/changelog.xml

[Rémy Maucherat]

2014-06-06 18:14 GMT+02:00 :

> Author: remm
> Date: Fri Jun  6 16:14:44 2014
> New Revision: 1600950
>
> URL: http://svn.apache.org/r1600950
> Log:
> Try removing the beta tag from the NIO2 connector to get more feedback,
> after the latest round of fixes. In practice since it is not used by
> default, it doesn't change much.
>

So if someone wants to keep it as beta, let me know before the tag.

Rémy

svn commit: r1600955 - in /tomcat/trunk: java/org/apache/catalina/servlets/WebdavServlet.java webapps/docs/changelog.xml

[kkolinko]

Author: kkolinko
Date: Fri Jun  6 16:24:32 2014
New Revision: 1600955

URL: http://svn.apache.org/r1600955
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56600
Fix missing "return;" statements after sendError():
do not waste time generating response for PROPFIND requests that contain broken 
XML document.

Modified:
tomcat/trunk/java/org/apache/catalina/servlets/WebdavServlet.java
tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/servlets/WebdavServlet.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/servlets/WebdavServlet.java?rev=1600955&r1=1600954&r2=1600955&view=diff
==
--- tomcat/trunk/java/org/apache/catalina/servlets/WebdavServlet.java (original)
+++ tomcat/trunk/java/org/apache/catalina/servlets/WebdavServlet.java Fri Jun  
6 16:24:32 2014
@@ -521,9 +521,11 @@ public class WebdavServlet
 } catch (SAXException e) {
 // Something went wrong - bad request
 resp.sendError(WebdavStatus.SC_BAD_REQUEST);
+return;
 } catch (IOException e) {
 // Something went wrong - bad request
 resp.sendError(WebdavStatus.SC_BAD_REQUEST);
+return;
 }
 }
 

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1600955&r1=1600954&r2=1600955&view=diff
==
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Jun  6 16:24:32 2014
@@ -126,6 +126,10 @@
 and TLD defined listeners are added via a different code path that
 already enforces the specification requirements. (markt)
   
+  
+56600: In WebdavServlet: Do not waste time generating
+response for broken PROPFIND requests. (kkolinko)
+  
 
   
   



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

svn commit: r1600956 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/servlets/WebdavServlet.java webapps/docs/changelog.xml

[kkolinko]

Author: kkolinko
Date: Fri Jun  6 16:30:50 2014
New Revision: 1600956

URL: http://svn.apache.org/r1600956
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56600
Fix missing "return;" statements after sendError():
do not waste time generating response for PROPFIND requests that contain broken 
XML document.

Merged r1600955 from tomcat/trunk.

Modified:
tomcat/tc7.0.x/trunk/   (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/WebdavServlet.java
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc7.0.x/trunk/
--
  Merged /tomcat/trunk:r1600955

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/WebdavServlet.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/WebdavServlet.java?rev=1600956&r1=1600955&r2=1600956&view=diff
==
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/WebdavServlet.java 
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/WebdavServlet.java 
Fri Jun  6 16:30:50 2014
@@ -563,9 +563,11 @@ public class WebdavServlet
 } catch (SAXException e) {
 // Something went wrong - bad request
 resp.sendError(WebdavStatus.SC_BAD_REQUEST);
+return;
 } catch (IOException e) {
 // Something went wrong - bad request
 resp.sendError(WebdavStatus.SC_BAD_REQUEST);
+return;
 }
 }
 

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1600956&r1=1600955&r2=1600956&view=diff
==
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Fri Jun  6 16:30:50 2014
@@ -85,6 +85,10 @@
 56588: Update deprecation of 
Context.addApplicationListener()
 methods according to changes in Tomcat 8. (kkolinko)
   
+  
+56600: In WebdavServlet: Do not waste time generating
+response for broken PROPFIND request. (kkolinko)
+  
 
   
   



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

svn commit: r1600957 - /tomcat/trunk/webapps/docs/changelog.xml

[kkolinko]

Author: kkolinko
Date: Fri Jun  6 16:31:53 2014
New Revision: 1600957

URL: http://svn.apache.org/r1600957
Log:
Correct a typo.

Modified:
tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1600957&r1=1600956&r2=1600957&view=diff
==
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Jun  6 16:31:53 2014
@@ -128,7 +128,7 @@
   
   
 56600: In WebdavServlet: Do not waste time generating
-response for broken PROPFIND requests. (kkolinko)
+response for broken PROPFIND request. (kkolinko)
   
 
   



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

svn commit: r1600958 - /tomcat/tc6.0.x/trunk/STATUS.txt

[kkolinko]

Author: kkolinko
Date: Fri Jun  6 16:34:49 2014
New Revision: 1600958

URL: http://svn.apache.org/r1600958
Log:
proposal

Modified:
tomcat/tc6.0.x/trunk/STATUS.txt

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1600958&r1=1600957&r2=1600958&view=diff
==
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Jun  6 16:34:49 2014
@@ -36,6 +36,12 @@ PATCHES PROPOSED TO BACKPORT:
   +1: markt, kkolinko
   -1:
 
+* http://issues.apache.org/bugzilla/show_bug.cgi?id=56600
+  In WebdavServlet: Do not waste time generating response for broken PROPFIND 
request.
+  http://svn.apache.org/r1600956
+  +1: kkolinko
+  -1:
+
 
 PATCHES/ISSUES THAT ARE STALLED:
 



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

[Bug 56600] Missing "return" statements in WebdavServlet.doPropfind()

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=56600

Konstantin Kolinko  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED
 OS||All

--- Comment #1 from Konstantin Kolinko  ---
Fixed in Tomcat 8 and Tomcat 7 (r1600955, r1600956)
and will be in 8.0.9, 7.0.55 onwards.

Proposed for Tomcat 6.

-- 
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: svn commit: r1600950 - in /tomcat/trunk: java/org/apache/tomcat/util/net/Nio2Endpoint.java webapps/docs/changelog.xml

[Konstantin Kolinko]

2014-06-06 20:17 GMT+04:00 Rémy Maucherat :
> 2014-06-06 18:14 GMT+02:00 :
>
>> Author: remm
>> Date: Fri Jun  6 16:14:44 2014
>> New Revision: 1600950
>>
>> URL: http://svn.apache.org/r1600950
>> Log:
>> Try removing the beta tag from the NIO2 connector to get more feedback,
>> after the latest round of fixes. In practice since it is not used by
>> default, it doesn't change much.
>>
>
> So if someone wants to keep it as beta, let me know before the tag.

1. Doesn't it feel too early?

2. You only removed a startup warning. I think the documentation still
labels it as beta.

Best regards,
Konstantin Kolinko

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

[Bug 56573] [Spec?] Session.getRequestURI() doesn't return the protocol, port etc.

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=56573

Mark Thomas  changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|FIXED   |---

--- Comment #7 from Mark Thomas  ---
I've received reports that this change triggers a TCK failure. I'll therefore
be reverting this change.

-- 
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

svn commit: r1600960 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/tomcat/websocket/server/ test/org/apache/tomcat/websocket/server/ webapps/docs/

[markt]

Author: markt
Date: Fri Jun  6 16:49:21 2014
New Revision: 1600960

URL: http://svn.apache.org/r1600960
Log:
Revert fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=56573
The fix triggered a TCK failure

Removed:

tomcat/tc7.0.x/trunk/test/org/apache/tomcat/websocket/server/TestWsHandshakeRequest.java

tomcat/tc7.0.x/trunk/test/org/apache/tomcat/websocket/server/TesterUriServer.java
Modified:
tomcat/tc7.0.x/trunk/   (props changed)

tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties

tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc7.0.x/trunk/
--
  Reverse-merged /tomcat/trunk:r1600651,1600663
  Merged /tomcat/trunk:r1600833

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties?rev=1600960&r1=1600959&r2=1600960&view=diff
==
--- 
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties
 (original)
+++ 
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties
 Fri Jun  6 16:49:21 2014
@@ -31,8 +31,6 @@ uriTemplate.emptySegment=The path [{0}] 
 uriTemplate.invalidPath=The path [{0}] is not valid.
 uriTemplate.invalidSegment=The segment [{0}] is not valid in the provided path 
[{1}]
 
-wsHandshakeRequest.unknownScheme=The scheme [{0}] is not recognised. [http] or 
[https] is expected
-
 wsHttpUpgradeHandler.destroyFailed=Failed to close WebConnection while 
destroying the WebSocket HttpUpgradeHandler
 wsHttpUpgradeHandler.noPreInit=The preInit() method must be called to 
configure the WebSocket HttpUpgradeHandler before the container calls init(). 
Usually, this means the Servlet that created the WsHttpUpgradeHandler instance 
should also call preInit()
 

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java?rev=1600960&r1=1600959&r2=1600960&view=diff
==
--- 
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java
 (original)
+++ 
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java
 Fri Jun  6 16:49:21 2014
@@ -30,15 +30,11 @@ import java.util.Map.Entry;
 import javax.servlet.http.HttpServletRequest;
 import javax.websocket.server.HandshakeRequest;
 
-import org.apache.tomcat.util.res.StringManager;
-
 /**
  * Represents the request that this session was opened under.
  */
 public class WsHandshakeRequest implements HandshakeRequest {
 
-private static final StringManager sm = 
StringManager.getManager(Constants.PACKAGE_NAME);
-
 private final URI requestUri;
 private final Map> parameterMap;
 private final String queryString;
@@ -58,34 +54,11 @@ public class WsHandshakeRequest implemen
 httpSession = request.getSession(false);
 
 // URI
-// Based on request.getRequestURL() implementation
-StringBuilder sb = new StringBuilder();
-String scheme = request.getScheme();
-int port = request.getServerPort();
-if (port < 0)
-port = 80; // Work around java.net.URL bug
-
-if (scheme.equals("http")) {
-sb.append("ws");
-} else if (scheme.equals("https")) {
-sb.append("wss");
-} else {
-throw new IllegalArgumentException(
-sm.getString("wsHandshakeRequest.unknownScheme", scheme));
-}
-sb.append("://");
-sb.append(request.getServerName());
-if ((scheme.equals("http") && (port != 80))
-|| (scheme.equals("https") && (port != 443))) {
-sb.append(':');
-sb.append(port);
-}
-sb.append(request.getRequestURI());
+StringBuilder sb = new StringBuilder(request.getRequestURI());
 if (queryString != null) {
-sb.append('?');
+sb.append("?");
 sb.append(queryString);
 }
-
 try {
 requestUri = new URI(sb.toString());
 } catch (URISyntaxException e) {

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1600960&r1=1600959&r2=1600960&view=diff
==
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Fri Jun  6 16:49:21 2014
@@ -117,16 +117,6 @@
   

svn commit: r1600961 - in /tomcat/trunk: java/org/apache/tomcat/websocket/server/ test/org/apache/tomcat/websocket/server/ webapps/docs/

[markt]

Author: markt
Date: Fri Jun  6 16:50:24 2014
New Revision: 1600961

URL: http://svn.apache.org/r1600961
Log:
Revert fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=56573
The fix triggered a TCK failure

Removed:

tomcat/trunk/test/org/apache/tomcat/websocket/server/TestWsHandshakeRequest.java
tomcat/trunk/test/org/apache/tomcat/websocket/server/TesterUriServer.java
Modified:
tomcat/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties
tomcat/trunk/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java
tomcat/trunk/webapps/docs/changelog.xml

Modified: 
tomcat/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties?rev=1600961&r1=1600960&r2=1600961&view=diff
==
--- 
tomcat/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties 
(original)
+++ 
tomcat/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties 
Fri Jun  6 16:50:24 2014
@@ -29,8 +29,6 @@ uriTemplate.emptySegment=The path [{0}] 
 uriTemplate.invalidPath=The path [{0}] is not valid.
 uriTemplate.invalidSegment=The segment [{0}] is not valid in the provided path 
[{1}]
 
-wsHandshakeRequest.unknownScheme=The scheme [{0}] is not recognised. [http] or 
[https] is expected
-
 wsHttpUpgradeHandler.destroyFailed=Failed to close WebConnection while 
destroying the WebSocket HttpUpgradeHandler
 wsHttpUpgradeHandler.noPreInit=The preInit() method must be called to 
configure the WebSocket HttpUpgradeHandler before the container calls init(). 
Usually, this means the Servlet that created the WsHttpUpgradeHandler instance 
should also call preInit()
 

Modified: 
tomcat/trunk/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java?rev=1600961&r1=1600960&r2=1600961&view=diff
==
--- 
tomcat/trunk/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java 
(original)
+++ 
tomcat/trunk/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java 
Fri Jun  6 16:50:24 2014
@@ -30,15 +30,11 @@ import java.util.Map.Entry;
 import javax.servlet.http.HttpServletRequest;
 import javax.websocket.server.HandshakeRequest;
 
-import org.apache.tomcat.util.res.StringManager;
-
 /**
  * Represents the request that this session was opened under.
  */
 public class WsHandshakeRequest implements HandshakeRequest {
 
-private static final StringManager sm = 
StringManager.getManager(Constants.PACKAGE_NAME);
-
 private final URI requestUri;
 private final Map> parameterMap;
 private final String queryString;
@@ -58,34 +54,11 @@ public class WsHandshakeRequest implemen
 httpSession = request.getSession(false);
 
 // URI
-// Based on request.getRequestURL() implementation
-StringBuilder sb = new StringBuilder();
-String scheme = request.getScheme();
-int port = request.getServerPort();
-if (port < 0)
-port = 80; // Work around java.net.URL bug
-
-if (scheme.equals("http")) {
-sb.append("ws");
-} else if (scheme.equals("https")) {
-sb.append("wss");
-} else {
-throw new IllegalArgumentException(
-sm.getString("wsHandshakeRequest.unknownScheme", scheme));
-}
-sb.append("://");
-sb.append(request.getServerName());
-if ((scheme.equals("http") && (port != 80))
-|| (scheme.equals("https") && (port != 443))) {
-sb.append(':');
-sb.append(port);
-}
-sb.append(request.getRequestURI());
+StringBuilder sb = new StringBuilder(request.getRequestURI());
 if (queryString != null) {
-sb.append('?');
+sb.append("?");
 sb.append(queryString);
 }
-
 try {
 requestUri = new URI(sb.toString());
 } catch (URISyntaxException e) {

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1600961&r1=1600960&r2=1600961&view=diff
==
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Jun  6 16:50:24 2014
@@ -219,16 +219,6 @@
 Add more varied endpoints for echo testing. (remm)
   
   
-56573: Change the value returned by
-Session.getRequestURI() from the value obtained from
-HttpServletRequest.getRequestURI() plus query string to 
the
-value obtained from HttpServletRequest.getRequestURI() 
plus
-query string with the scheme changed to ws or wss as 

[Bug 56573] [Spec?] Session.getRequestURI() doesn't return the protocol, port etc.

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=56573

Mark Thomas  changed:

   What|Removed |Added

 Status|REOPENED|RESOLVED
 Resolution|--- |INVALID

--- Comment #8 from Mark Thomas  ---
The current behavior is required by the TCK.

-- 
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

svn commit: r1600963 - /tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java

[markt]

Author: markt
Date: Fri Jun  6 16:58:02 2014
New Revision: 1600963

URL: http://svn.apache.org/r1600963
Log:
Handle exceptions when trying to flush the response.

Modified:
tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java

Modified: tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java?rev=1600963&r1=1600962&r2=1600963&view=diff
==
--- tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java Fri Jun  
6 16:58:02 2014
@@ -80,8 +80,13 @@ public class ErrorReportValve extends Va
 
 if (response.isCommitted()) {
 if (response.isErrorAfterCommit()) {
-// Flush any data that is still to be written to the client
-response.flushBuffer();
+// Attempt to flush any data that is still to be written to the
+// client
+try {
+response.flushBuffer();
+} catch (Throwable t) {
+ExceptionUtils.handleThrowable(t);
+}
 // Close immediately to signal to the client that something 
went
 // wrong
 response.getCoyoteResponse().action(ActionCode.CLOSE_NOW, 
null);



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

Re: Error handling

[Mark Thomas]

On 06/06/2014 12:29, Konstantin Kolinko wrote:



> Reviewing r1600449 and followups.
> 
> // ErrorReportValve.java
> [[[
> if (response.isCommitted()) {
> if (response.isErrorAfterCommit()) {
> // Flush any data that is still to be written to the client
> response.flushBuffer();
> // Close immediately to signal to the client that something 
> went
> // wrong
> response.getCoyoteResponse().action(ActionCode.CLOSE_NOW, 
> null);
> }
> return;
> }
> ]]]
> 
> 1. If original error was due to I/O failure, then I expect
> "response.flushBuffer()" call to fail with an IOException.
> 
> In ErrorReportValve#report() any IOException is silently swallowed. So
> I think we can swallow it here.

Agreed. Fixed.


> // ErrorReportValve.java
> [[[
> if (throwable != null) {
> // Make sure that the necessary methods have been called on the
> // response. (It is possible a component may just have set the
> // Throwable. Tomcat won't do that but other components might.)
> // These are safe to call at this point as we know that the 
> response
> // has not been committed.
> response.reset();
> response.sendError(
> HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
> }
> 
> // One way or another, response.sendError() will have been called 
> before
> // execution reaches this point and suspended the response. Need to
> // reverse that so this valve can write to the response.
> response.setSuspended(false);
> 
> try {
> report(request, response, throwable);
> } catch (Throwable tt) {
> ExceptionUtils.handleThrowable(tt);
> }
> ]]]
> 
> I know that the above calls were essentially present before these
> changes. Looking at them I have the following thoughts:
> 
> 2. Looking at "if (throwable != null)".
> The error might have already been handled either by custom error page
> (StandardHostValve.custom(...)) or if there are several
> ErrorReportValve valves in the chain.
> 
> In both cases the RequestDispatcher.ERROR_EXCEPTION attribute is not cleared.
> 
> The StandardHostValve saves us from this by always calling
> response.flushBuffer();
> just after invoking its custom() method. Thus "if
> response.isCommitted()" branch is triggered and the error report valve
> invocation returns early.
> 
> ErrorReportValve.report() can be updated to call flushBuffer() as
> well. Otherwise I think if there are several ErrorReportValve valves
> the "if throwable != null" branch will reset the response and it will
> be generated a-new.
> 
> Though if there is no other good reason to flush, then it looks like a
> hack (in StandardHostValve as well). (A known drawback of
> flushBuffer() is that it causes chunked encoding with HTTP/1.1, even
> if the whole response fits in a buffer).
> 
> I wonder if those flushBuffer() calls can be removed.

finishResponse() would avoid chunking if the response fitted into the
buffer.

> 3. Some Tomcat components set RequestDispatcher.ERROR_EXCEPTION
> attribute without setting any other error flags.
> 
> E.g. o.a.c.connector.Request#notifyAttributeAssigned),
> #notifyAttributeRemoved().
> 
> I think this is the only use case where "if (throwable != null)"
> branch and response.reset() call there are justified.
> 
> If response have been already committed the "if
> response.isCommitted()" branch will be triggered, but not "if
> response.isErrorAfterCommit()", as error flag has not been set.
> 
> So I interpret this use case as "Some error was detected, but it is
> not fatal. If response have not been committed then report it. If it
> has been committed, ignore it".
> 
> 
> For 3. I think maybe do the following:
> Replace "if (throwable != null)"
> with "if (throwable != null && !response.isError())"
> 
> If the above replacement is done then I think that maybe those
> flushBuffer() calls in StandardHostValve mentioned in "2." can be
> removed.

Ah. Interesting. I'll take a closer look at that.

Mark


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

Re: Error handling

[Mark Thomas]

On 06/06/2014 18:01, Mark Thomas wrote:
> On 06/06/2014 12:29, Konstantin Kolinko wrote:
> 
> 
> 
>> Reviewing r1600449 and followups.
>>
>> // ErrorReportValve.java
>> [[[
>> if (response.isCommitted()) {
>> if (response.isErrorAfterCommit()) {
>> // Flush any data that is still to be written to the client
>> response.flushBuffer();
>> // Close immediately to signal to the client that something 
>> went
>> // wrong
>> response.getCoyoteResponse().action(ActionCode.CLOSE_NOW, 
>> null);
>> }
>> return;
>> }
>> ]]]
>>
>> 1. If original error was due to I/O failure, then I expect
>> "response.flushBuffer()" call to fail with an IOException.
>>
>> In ErrorReportValve#report() any IOException is silently swallowed. So
>> I think we can swallow it here.
> 
> Agreed. Fixed.
> 
> 
>> // ErrorReportValve.java
>> [[[
>> if (throwable != null) {
>> // Make sure that the necessary methods have been called on the
>> // response. (It is possible a component may just have set the
>> // Throwable. Tomcat won't do that but other components might.)
>> // These are safe to call at this point as we know that the 
>> response
>> // has not been committed.
>> response.reset();
>> response.sendError(
>> HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
>> }
>>
>> // One way or another, response.sendError() will have been called 
>> before
>> // execution reaches this point and suspended the response. Need to
>> // reverse that so this valve can write to the response.
>> response.setSuspended(false);
>>
>> try {
>> report(request, response, throwable);
>> } catch (Throwable tt) {
>> ExceptionUtils.handleThrowable(tt);
>> }
>> ]]]
>>
>> I know that the above calls were essentially present before these
>> changes. Looking at them I have the following thoughts:
>>
>> 2. Looking at "if (throwable != null)".
>> The error might have already been handled either by custom error page
>> (StandardHostValve.custom(...)) or if there are several
>> ErrorReportValve valves in the chain.
>>
>> In both cases the RequestDispatcher.ERROR_EXCEPTION attribute is not cleared.
>>
>> The StandardHostValve saves us from this by always calling
>> response.flushBuffer();
>> just after invoking its custom() method. Thus "if
>> response.isCommitted()" branch is triggered and the error report valve
>> invocation returns early.
>>
>> ErrorReportValve.report() can be updated to call flushBuffer() as
>> well. Otherwise I think if there are several ErrorReportValve valves
>> the "if throwable != null" branch will reset the response and it will
>> be generated a-new.
>>
>> Though if there is no other good reason to flush, then it looks like a
>> hack (in StandardHostValve as well). (A known drawback of
>> flushBuffer() is that it causes chunked encoding with HTTP/1.1, even
>> if the whole response fits in a buffer).
>>
>> I wonder if those flushBuffer() calls can be removed.
> 
> finishResponse() would avoid chunking if the response fitted into the
> buffer.
> 
>> 3. Some Tomcat components set RequestDispatcher.ERROR_EXCEPTION
>> attribute without setting any other error flags.
>>
>> E.g. o.a.c.connector.Request#notifyAttributeAssigned),
>> #notifyAttributeRemoved().
>>
>> I think this is the only use case where "if (throwable != null)"
>> branch and response.reset() call there are justified.
>>
>> If response have been already committed the "if
>> response.isCommitted()" branch will be triggered, but not "if
>> response.isErrorAfterCommit()", as error flag has not been set.
>>
>> So I interpret this use case as "Some error was detected, but it is
>> not fatal. If response have not been committed then report it. If it
>> has been committed, ignore it".
>>
>>
>> For 3. I think maybe do the following:
>> Replace "if (throwable != null)"
>> with "if (throwable != null && !response.isError())"
>>
>> If the above replacement is done then I think that maybe those
>> flushBuffer() calls in StandardHostValve mentioned in "2." can be
>> removed.
> 
> Ah. Interesting. I'll take a closer look at that.

Looking at the execution flow, they could be removed but at the cost of
the ErrorReportValve doing more processing before it decided it wasn't
going to do anything. I think on reflection I'll go the finishResponse()
route.

Mark


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

svn commit: r1600965 - in /tomcat/trunk/java/org/apache/catalina: core/StandardHostValve.java valves/ErrorReportValve.java

[markt]

Author: markt
Date: Fri Jun  6 17:14:19 2014
New Revision: 1600965

URL: http://svn.apache.org/r1600965
Log:
kkolinko review
Use finishResponse() rather than flushBuffer to avoid chunking if not necessary.
Add finishResponse() to ErrorReportValve.report()
Don;t try and reset the response if it has already been marked as in error

Modified:
tomcat/trunk/java/org/apache/catalina/core/StandardHostValve.java
tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java

Modified: tomcat/trunk/java/org/apache/catalina/core/StandardHostValve.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardHostValve.java?rev=1600965&r1=1600964&r2=1600965&view=diff
==
--- tomcat/trunk/java/org/apache/catalina/core/StandardHostValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardHostValve.java Fri Jun  
6 17:14:19 2014
@@ -290,7 +290,7 @@ final class StandardHostValve extends Va
  request.getRequestURI());
 if (custom(request, response, errorPage)) {
 try {
-response.flushBuffer();
+response.finishResponse();
 } catch (ClientAbortException e) {
 // Ignore
 } catch (IOException e) {
@@ -366,7 +366,7 @@ final class StandardHostValve extends Va
   realError.getClass());
 if (custom(request, response, errorPage)) {
 try {
-response.flushBuffer();
+response.finishResponse();
 } catch (IOException e) {
 container.getLogger().warn("Exception Processing " + 
errorPage, e);
 }

Modified: tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java?rev=1600965&r1=1600964&r2=1600965&view=diff
==
--- tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java Fri Jun  
6 17:14:19 2014
@@ -101,7 +101,7 @@ public class ErrorReportValve extends Va
 return;
 }
 
-if (throwable != null) {
+if (throwable != null && !response.isError()) {
 // Make sure that the necessary methods have been called on the
 // response. (It is possible a component may just have set the
 // Throwable. Tomcat won't do that but other components might.)
@@ -274,6 +274,7 @@ public class ErrorReportValve extends Va
 // If writer is null, it's an indication that the response has
 // been hard committed already, which should never happen
 writer.write(sb.toString());
+response.finishResponse();
 }
 } catch (IOException e) {
 // Ignore



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

Re: svn commit: r1600950 - in /tomcat/trunk: java/org/apache/tomcat/util/net/Nio2Endpoint.java webapps/docs/changelog.xml

[Rémy Maucherat]

2014-06-06 18:39 GMT+02:00 Konstantin Kolinko :

> 1. Doesn't it feel too early?
>

Maybe, but tests pass and no reports.

>
> 2. You only removed a startup warning. I think the documentation still
> labels it as beta.
>

This could be a good middle ground.

Rémy

buildbot success in ASF Buildbot on tomcat-7-trunk

[buildbot]

The Buildbot has detected a restored build on builder tomcat-7-trunk while 
building ASF Buildbot.
Full details are available at:
 http://ci.apache.org/builders/tomcat-7-trunk/builds/101

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: bb-vm_ubuntu

Build Reason: scheduler
Build Source Stamp: [branch tomcat/tc7.0.x/trunk] 1600960
Blamelist: markt

Build succeeded!

sincerely,
 -The Buildbot




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

svn commit: r1600978 - /tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java

[markt]

Author: markt
Date: Fri Jun  6 18:38:09 2014
New Revision: 1600978

URL: http://svn.apache.org/r1600978
Log:
Clean-up. No functional change.

Modified:
tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java

Modified: 
tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java?rev=1600978&r1=1600977&r2=1600978&view=diff
==
--- tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java 
Fri Jun  6 18:38:09 2014
@@ -14,7 +14,6 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  */
-
 package org.apache.coyote.http11.filters;
 
 import java.io.EOFException;
@@ -41,14 +40,12 @@ public class ChunkedInputFilter implemen
 
 // -- Constants
 
-
 protected static final String ENCODING_NAME = "chunked";
 protected static final ByteChunk ENCODING = new ByteChunk();
 
 
 // - Static Initializer
 
-
 static {
 ENCODING.setBytes(ENCODING_NAME.getBytes(StandardCharsets.ISO_8859_1),
 0, ENCODING_NAME.length());
@@ -57,7 +54,6 @@ public class ChunkedInputFilter implemen
 
 // - Instance Variables
 
-
 /**
  * Next buffer in the pipeline.
  */
@@ -105,6 +101,7 @@ public class ChunkedInputFilter implemen
  */
 protected final ByteChunk trailingHeaders = new ByteChunk();
 
+
 /**
  * Flag set to true if the next call to doRead() must parse a CRLF pair
  * before doing anything else.
@@ -129,6 +126,7 @@ public class ChunkedInputFilter implemen
  */
 private final int maxTrailerSize;
 
+
 /**
  * Size of extensions processed for this request.
  */
@@ -136,14 +134,15 @@ public class ChunkedInputFilter implemen
 
 
 // --- Constructors
+
 public ChunkedInputFilter(int maxTrailerSize, int maxExtensionSize) {
 this.trailingHeaders.setLimit(maxTrailerSize);
 this.maxExtensionSize = maxExtensionSize;
 this.maxTrailerSize = maxTrailerSize;
 }
 
-//  InputBuffer Methods
 
+//  InputBuffer Methods
 
 /**
  * Read bytes.
@@ -155,11 +154,11 @@ public class ChunkedInputFilter implemen
  * control, the returned value should be -1.
  */
 @Override
-public int doRead(ByteChunk chunk, Request req)
-throws IOException {
+public int doRead(ByteChunk chunk, Request req) throws IOException {
 
-if (endChunk)
+if (endChunk) {
 return -1;
+}
 
 if(needCRLFParse) {
 needCRLFParse = false;
@@ -206,13 +205,11 @@ public class ChunkedInputFilter implemen
 }
 
 return result;
-
 }
 
 
 //  InputFilter Methods
 
-
 /**
  * Read the content length from the request.
  */
@@ -226,8 +223,7 @@ public class ChunkedInputFilter implemen
  * End the current request.
  */
 @Override
-public long end()
-throws IOException {
+public long end() throws IOException {
 
 // Consume extra bytes : parse the stream until the end chunk is found
 while (doRead(readChunk, null) >= 0) {
@@ -235,8 +231,7 @@ public class ChunkedInputFilter implemen
 }
 
 // Return the number of extra bytes which were consumed
-return (lastValid - pos);
-
+return lastValid - pos;
 }
 
 
@@ -245,7 +240,7 @@ public class ChunkedInputFilter implemen
  */
 @Override
 public int available() {
-return (lastValid - pos);
+return lastValid - pos;
 }
 
 
@@ -286,12 +281,10 @@ public class ChunkedInputFilter implemen
 
 // -- Protected Methods
 
-
 /**
  * Read bytes from the previous buffer.
  */
-protected int readBytes()
-throws IOException {
+protected int readBytes() throws IOException {
 
 int nRead = buffer.doRead(readChunk, null);
 pos = readChunk.getStart();
@@ -299,7 +292,6 @@ public class ChunkedInputFilter implemen
 buf = readChunk.getBytes();
 
 return nRead;
-
 }
 
 
@@ -314,8 +306,7 @@ public class ChunkedInputFilter implemen
  * digits. We should not parse F23IAMGONNAMESSTHISUP34CRLF as a valid
  * header according to the spec.
  */
-protected boolean parseChunkHeader()
-throws IOException {
+protected boolean pars

svn commit: r1600984 - in /tomcat/trunk/java/org/apache/coyote/http11/filters: ChunkedInputFilter.java LocalStrings.properties

[markt]

Author: markt
Date: Fri Jun  6 19:14:07 2014
New Revision: 1600984

URL: http://svn.apache.org/r1600984
Log:
i18n for ChunkedInputFilter error message
Add error flag to allow subsequent attempts at reading after an error to fail 
fast

Added:
tomcat/trunk/java/org/apache/coyote/http11/filters/LocalStrings.properties  
 (with props)
Modified:
tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java

Modified: 
tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java?rev=1600984&r1=1600983&r2=1600984&view=diff
==
--- tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java 
Fri Jun  6 19:14:07 2014
@@ -28,6 +28,7 @@ import org.apache.tomcat.util.buf.ByteCh
 import org.apache.tomcat.util.buf.HexUtils;
 import org.apache.tomcat.util.buf.MessageBytes;
 import org.apache.tomcat.util.http.MimeHeaders;
+import org.apache.tomcat.util.res.StringManager;
 
 /**
  * Chunked input filter. Parses chunked data according to
@@ -37,6 +38,9 @@ import org.apache.tomcat.util.http.MimeH
  */
 public class ChunkedInputFilter implements InputFilter {
 
+private static final StringManager sm = StringManager.getManager(
+ChunkedInputFilter.class.getPackage().getName());
+
 
 // -- Constants
 
@@ -133,6 +137,12 @@ public class ChunkedInputFilter implemen
 private long extensionSize;
 
 
+/**
+ * Flag that indicates if an error has occurred.
+ */
+private boolean error;
+
+
 // --- Constructors
 
 public ChunkedInputFilter(int maxTrailerSize, int maxExtensionSize) {
@@ -155,6 +165,7 @@ public class ChunkedInputFilter implemen
  */
 @Override
 public int doRead(ByteChunk chunk, Request req) throws IOException {
+checkError();
 
 if (endChunk) {
 return -1;
@@ -167,7 +178,7 @@ public class ChunkedInputFilter implemen
 
 if (remaining <= 0) {
 if (!parseChunkHeader()) {
-throw new IOException("Invalid chunk header");
+
throwIOException(sm.getString("chunkedInputFilter.invalidHeader"));
 }
 if (endChunk) {
 parseEndChunk();
@@ -179,8 +190,7 @@ public class ChunkedInputFilter implemen
 
 if (pos >= lastValid) {
 if (readBytes() < 0) {
-throw new IOException(
-"Unexpected end of stream whilst reading request 
body");
+throwIOException(sm.getString("chunkedInputFilter.eos"));
 }
 }
 
@@ -224,6 +234,7 @@ public class ChunkedInputFilter implemen
  */
 @Override
 public long end() throws IOException {
+checkError();
 
 // Consume extra bytes : parse the stream until the end chunk is found
 while (doRead(readChunk, null) >= 0) {
@@ -266,6 +277,7 @@ public class ChunkedInputFilter implemen
 trailingHeaders.recycle();
 trailingHeaders.setLimit(maxTrailerSize);
 extensionSize = 0;
+error = false;
 }
 
 
@@ -279,6 +291,12 @@ public class ChunkedInputFilter implemen
 }
 
 
+@Override
+public boolean isFinished() {
+return endChunk;
+}
+
+
 // -- Protected Methods
 
 /**
@@ -346,7 +364,7 @@ public class ChunkedInputFilter implemen
 // validated. Currently it is simply ignored.
 extensionSize++;
 if (maxExtensionSize > -1 && extensionSize > maxExtensionSize) 
{
-throw new IOException("maxExtensionSize exceeded");
+
throwIOException(sm.getString("chunkedInputFilter.maxExtension"));
 }
 }
 
@@ -387,20 +405,23 @@ public class ChunkedInputFilter implemen
 
 while (!eol) {
 if (pos >= lastValid) {
-if (readBytes() <= 0)
-throw new IOException("Invalid CRLF");
+if (readBytes() <= 0) {
+
throwIOException(sm.getString("chunkedInputFilter.invalidCrlfNoData"));
+}
 }
 
 if (buf[pos] == Constants.CR) {
-if (crfound) throw new IOException("Invalid CRLF, two CR 
characters encountered.");
+if (crfound) {
+
throwIOException(sm.getString("chunkedInputFilter.invalidCrlfCRCR"));
+}
 crfound = true;
 } else if (buf[pos] == Constants.LF) {
 if (!tolerant && !crfound) {
-throw new IOException("Invalid CRLF, no CR character 
enc

[Bug 55975] Inconsistent escaping applied to V0 cookie values

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=55975

Mark Thomas  changed:

   What|Removed |Added

 Status|REOPENED|RESOLVED
 Resolution|--- |FIXED

--- Comment #3 from Mark Thomas  ---
This is fixed. Those bug references were wrong.

-- 
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

svn commit: r1600988 - /tomcat/trunk/test/org/apache/tomcat/util/http/TestSetCookieSupportSeparatorsAllowed.java

[markt]

Author: markt
Date: Fri Jun  6 19:33:36 2014
New Revision: 1600988

URL: http://svn.apache.org/r1600988
Log:
Correct bug reference. These are all V1 cookies. bug 55975 is a V0 cookie issue

Modified:

tomcat/trunk/test/org/apache/tomcat/util/http/TestSetCookieSupportSeparatorsAllowed.java

Modified: 
tomcat/trunk/test/org/apache/tomcat/util/http/TestSetCookieSupportSeparatorsAllowed.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/http/TestSetCookieSupportSeparatorsAllowed.java?rev=1600988&r1=1600987&r2=1600988&view=diff
==
--- 
tomcat/trunk/test/org/apache/tomcat/util/http/TestSetCookieSupportSeparatorsAllowed.java
 (original)
+++ 
tomcat/trunk/test/org/apache/tomcat/util/http/TestSetCookieSupportSeparatorsAllowed.java
 Fri Jun  6 19:33:36 2014
@@ -164,7 +164,7 @@ public class TestSetCookieSupportSeparat
 Assert.assertEquals("foo=\"a\\\"b\"; Version=1", 
SetCookieSupport.generateHeader(cookie));
 }
 
-@Ignore("bug 55975")
+@Ignore("bug 55984")
 @Test
 public void v1ValueContainsNonV0Separator() {
 Cookie cookie = new Cookie("foo", "a()<>@,;:\\\"/[]?={}b");
@@ -173,7 +173,7 @@ public class TestSetCookieSupportSeparat
 Assert.assertEquals("foo=\"a()<>@,;:\\\"/[]?={}b\"; Version=1", 
SetCookieSupport.generateHeader(cookie));
 }
 
-@Ignore("bug 55975")
+@Ignore("bug 55984")
 @Test
 public void v1ValueContainsBackslash() {
 Cookie cookie = new Cookie("foo", "a\\b");
@@ -183,7 +183,7 @@ public class TestSetCookieSupportSeparat
 }
 
 
-@Ignore("bug 55975")
+@Ignore("bug 55984")
 @Test
 public void v1ValueContainsBackslashAndQuote() {
 Cookie cookie = new Cookie("foo", "a\"b\\c");



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

buildbot success in ASF Buildbot on tomcat-trunk

[buildbot]

The Buildbot has detected a restored build on builder tomcat-trunk while 
building ASF Buildbot.
Full details are available at:
 http://ci.apache.org/builders/tomcat-trunk/builds/156

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: bb-vm_ubuntu

Build Reason: scheduler
Build Source Stamp: [branch tomcat/trunk] 1600988
Blamelist: markt

Build succeeded!

sincerely,
 -The Buildbot




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