DO NOT REPLY [Bug 44620] infinit loop in nio connector code
https://issues.apache.org/bugzilla/show_bug.cgi?id=44620 --- Comment #5 from tangy [EMAIL PROTECTED] 2008-03-21 00:02:24 PST --- Sorry for that I do not express it clearly(We made some mistake in it). Since our application will cause the following problem: 2008-3-21 14:54:12 org.apache.catalina.connector.CoyoteAdapter service 严重: An exception or error occurred in the container during the request processing java.lang.ArrayIndexOutOfBoundsException: 8192 at org.apache.coyote.http11.InternalNioOutputBuffer.write(InternalNioOutputBuffer.java:734) at org.apache.coyote.http11.InternalNioOutputBuffer.write(InternalNioOutputBuffer.java:641) at org.apache.coyote.http11.InternalNioOutputBuffer.sendHeader(InternalNioOutputBuffer.java:507) at org.apache.coyote.http11.Http11NioProcessor.prepareResponse(Http11NioProcessor.java:1707) at org.apache.coyote.http11.Http11NioProcessor.action(Http11NioProcessor.java:1023) at org.apache.coyote.Response.action(Response.java:183) at org.apache.coyote.Response.sendHeaders(Response.java:379) at org.apache.catalina.connector.OutputBuffer.doFlush(OutputBuffer.java:305) at org.apache.catalina.connector.OutputBuffer.close(OutputBuffer.java:273) at org.apache.catalina.connector.Response.finishResponse(Response.java:492) at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:310) at org.apache.coyote.http11.Http11NioProcessor.process(Http11NioProcessor.java:879) at org.apache.coyote.http11.Http11NioProtocol$Http11ConnectionHandler.process(Http11NioProtocol.java:719) at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:2080) at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:885) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:907) at java.lang.Thread.run(Thread.java:619) 2008-3-21 14:54:12 org.apache.coyote.http11.Http11NioProcessor endRequest 严重: Error finishing response java.lang.ArrayIndexOutOfBoundsException at java.lang.System.arraycopy(Native Method) at org.apache.coyote.http11.InternalNioOutputBuffer.write(InternalNioOutputBuffer.java:703) at org.apache.coyote.http11.InternalNioOutputBuffer.sendStatus(InternalNioOutputBuffer.java:460) at org.apache.coyote.http11.Http11NioProcessor.prepareResponse(Http11NioProcessor.java:1696) at org.apache.coyote.http11.Http11NioProcessor.action(Http11NioProcessor.java:1023) at org.apache.coyote.Response.action(Response.java:181) at org.apache.coyote.http11.InternalNioOutputBuffer.endRequest(InternalNioOutputBuffer.java:382) at org.apache.coyote.http11.Http11NioProcessor.endRequest(Http11NioProcessor.java:977) at org.apache.coyote.http11.Http11NioProcessor.process(Http11NioProcessor.java:913) at org.apache.coyote.http11.Http11NioProtocol$Http11ConnectionHandler.process(Http11NioProtocol.java:719) at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:2080) at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:885) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:907) at java.lang.Thread.run(Thread.java:619) we changed the server.xml and set maxHttpHeaderSize=1024 , and which cause dead loop and cost 100%CPU. the detailed stack trace is below(tomcat 6.0.16): Daemon Thread [catalina-exec-4] (Suspended) InternalNioOutputBuffer.flushBuffer() line: 768 InternalNioOutputBuffer.addToBB(byte[], int, int) line: 616 InternalNioOutputBuffer.commit() line: 608 Http11NioProcessor.action(ActionCode, Object) line: 1024 Response.action(ActionCode, Object) line: 183 Response.sendHeaders() line: 379 OutputBuffer.doFlush(boolean) line: 305 OutputBuffer.close() line: 273 Response.finishResponse() line: 492 CoyoteAdapter.service(Request, Response) line: 310 Http11NioProcessor.process(NioChannel) line: 879 Http11NioProtocol$Http11ConnectionHandler.process(NioChannel) line: 719 NioEndpoint$SocketProcessor.run() line: 2080 ThreadPoolExecutor$Worker.runTask(Runnable) line: 885 ThreadPoolExecutor$Worker.run() line: 907 Thread.run() line: 619 At last we also increased the socket.appWriteBufSize=10240 and solved the problem. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
DO NOT REPLY [Bug 44620] infinit loop in nio connector code
https://issues.apache.org/bugzilla/show_bug.cgi?id=44620 --- Comment #6 from tangy [EMAIL PROTECTED] 2008-03-21 00:04:36 PST --- sorry for typed mistake: we changed the server.xml and set maxHttpHeaderSize=1024 , should be we changed the server.xml and set maxHttpHeaderSize=10240 , -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Cookies are broken in 6.0.16?
Hi all, This is a follow up to thread Cookies are broken in 6.0.16? http://marc.info/?t=12025394451r=2w=2 Before submitting a bug report on the matter exposed below I wanted to open a discussion with Tomcat developpers about it. Summary of the above thread : - Problem : If a webapp is using an equal sign in the value of a cookie, Tomcat alter the cookie value. - Explanation : For security reason a change has been introduced in 5.5.26 and 6.0.16 to cookie management regarding the equal sign. - Solution suggested by Filip Hanik in the thread: All applications should be modified to use cookie.setVersion(1) if they want to use equal sign in their value. Using version 1 cookie surrounds all cookies attributes with quotes and leave the value intact. First of all, I'd like to point out that breaking existing cookie behavior and requiring applications to be updated in order to work properly is probably not the best choice ... Clients tend to think twice before upgrading their application to a more recent version and it is not a process they want to perform every day... In our webapp, we use an equal sign in a cookie's value. Even though all other application server we use (BEA Weblogic, IBM WebSphere) do not have the above problem. I have decided not to brag too much about this decision for Tomcat and I was in the process of updating our CMS to use cookie version 1... but... ... it does not work with IE 7. Why ? Because a path is specified in our cookie. And Tomcat also surrounds the path with quotes, which is not accepted by IE 7 (probably other version of IE too, I have not tested yet). Here is a simple test case. Place this jsp in a directory subdir inside a ROOT webapp. % Cookie cookie1 = new Cookie(Cookie1, Hello World 1!); Cookie cookie2 = new Cookie(Cookie2, Hello World 2!); cookie2.setVersion(1); Cookie cookie3 = new Cookie(Cookie3, Hello World 3!); cookie3.setPath(/subdir); Cookie cookie4 = new Cookie(Cookie4, Hello World 4!); cookie4.setPath(/subdir); cookie4.setVersion(1); response.addCookie(cookie1); response.addCookie(cookie2); response.addCookie(cookie3); response.addCookie(cookie4); % Here is the Tomcat response will output : Set-Cookie: Cookie1=Hello World 1! Set-Cookie: Cookie2=Hello World 2!; Version=1 Set-Cookie: Cookie3=Hello World 3!; Path=/subdir Set-Cookie: Cookie4=Hello World 4!; Version=1; Path=/subdir == IE 7 does not send back the cookie #4. If using fiddler, we alter the response that way : Set-Cookie: Cookie1=Hello World 1! Set-Cookie: Cookie2=Hello World 2!; Version=1 Set-Cookie: Cookie3=Hello World 3!; Path=/subdir Set-Cookie: Cookie4=Hello World 4!; Version=1; Path=/subdir == IE 7 accepts all the cookie I can understand the security reasons behind the modification requiring quotes around cookies' attribute value. So my questions are : - What could be done in Tomcat to fix this ? - Is the security problem real if it is why do other application server vendors do not have this behavior ? - As it break older application and it is not compatible with IE 7 in some circumstances. Do you really think this modification should be kept that way ? Thanks in advance for your replies! Best regards, Olivier Jaquemet - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Cookies are broken in 6.0.16?
Olivier Jaquemet wrote: So my questions are : - What could be done in Tomcat to fix this ? This has been discussed extensively on the dev list and there is a patch proposed. See the archives for details. - Is the security problem real if it is why do other application server vendors do not have this behavior ? Yes the problem is real. It won't affect every installation or every application but it does affect some. As for the other app servers: don't know - haven't tested them. - As it break older application and it is not compatible with IE 7 in some circumstances. Do you really think this modification should be kept that way ? No. Again, see the discussion in the archives. Mark - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Cookies are broken in 6.0.16?
Thank you for your answer Mark. Accept my apologies for bothering you with already fixed issues. I had not found the appropriate discussions in the archive. Olivier Mark Thomas wrote: Olivier Jaquemet wrote: So my questions are : - What could be done in Tomcat to fix this ? This has been discussed extensively on the dev list and there is a patch proposed. See the archives for details. - Is the security problem real if it is why do other application server vendors do not have this behavior ? Yes the problem is real. It won't affect every installation or every application but it does affect some. As for the other app servers: don't know - haven't tested them. - As it break older application and it is not compatible with IE 7 in some circumstances. Do you really think this modification should be kept that way ? No. Again, see the discussion in the archives. Mark - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: renaming jar files
Costin Manolache wrote: What's the current status of commons-logging versus juli/logging ? Do we still need the dep ? in 5.5 we still support commoons logging, in 6.0, no more unless you build the adapter from the extras package Speaking of deps - I'm completely confused by tomcat-dbcp.jar - it seems to break the build ( 'download' ) with JDK1.6, yet I can't find that's because DBCP implements interfaces, and only implements the java.sql/javax.sql up to JDK 1.5. any usage in the code - and just removing it doesn't seem to hurt in any way. it's being used, removing it would prolly upset some users, as the DBCP is the core for the tomcat connection pooling. Am I missing something - the jar name is referenced if few places, and there is a class name in Constants. I assume some JDBC connector stuff would not work so well unless someone install the package. Costin On 3/20/08, Filip Hanik - Dev Lists [EMAIL PROTECTED] wrote: when the commons-logging jar files got updated, they also got renamed, we missed a few spots, some files in bin and conf still reference the old name, like catalina.policy // These permissions apply to the commons-logging API grant codeBase file:${catalina.home}/bin/commons-logging-api.jar { permission java.security.AllPermission; }; Filip - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
DO NOT REPLY [Bug 44494] Requests greater than 8k being truncated.
https://issues.apache.org/bugzilla/show_bug.cgi?id=44494 --- Comment #17 from Filip Hanik [EMAIL PROTECTED] 2008-03-21 08:48:11 PST --- (In reply to comment #15) Remy and Filip, thank you for immediate checks. Yes, the logic is same in all of them. (In reply to comment #11, #12) I tested the patch on 5.5 and it didn't work, I will look deeper into it I also tested the trunk patch, and couldn't get it to work properly I'm sorry, I might have made a mistake in something. I will check them more. (In reply to comment #14) if we are streaming a few megabytes of data, tomcat would have to cache all that data in order to support mark properly. If it says more in detail, Tomcat would have to cache data only after the mark() method calling, and Tomcat would have to cache characters up to only the size that the application specified with the method. yes, only after mark has been called, but if you set a readAheadLimit of 10MB, one has to cache that. Of course, we can choose to support these methods or not to support. I think it's wonderful if Tomcat can support that. yes, it's not a question about supporting or not supporting, it's where in the code you support that. with the bytechunk/charchunk the byte arrays referenced left and right, it makes it so much harder. I'll create a little POC to show how simpl it it is And I'm believing that the implementation of Tomcat has enough possibility. goes against the KISS principle :) -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Cookies are broken in 6.0.16?
One last question Mark... The tc6.6.x trunk has the patch and it works fine, but not yet the tc5.5.x. Is it planned for the next 5.5.x release ? Olivier Jaquemet wrote: Thank you for your answer Mark. Accept my apologies for bothering you with already fixed issues. I had not found the appropriate discussions in the archive. Olivier Mark Thomas wrote: Olivier Jaquemet wrote: So my questions are : - What could be done in Tomcat to fix this ? This has been discussed extensively on the dev list and there is a patch proposed. See the archives for details. - Is the security problem real if it is why do other application server vendors do not have this behavior ? Yes the problem is real. It won't affect every installation or every application but it does affect some. As for the other app servers: don't know - haven't tested them. - As it break older application and it is not compatible with IE 7 in some circumstances. Do you really think this modification should be kept that way ? No. Again, see the discussion in the archives. Mark - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Cookies are broken in 6.0.16?
even the 6.0.x needs one more patch in order to be backwards compatible Filip Olivier Jaquemet wrote: One last question Mark... The tc6.6.x trunk has the patch and it works fine, but not yet the tc5.5.x. Is it planned for the next 5.5.x release ? Olivier Jaquemet wrote: Thank you for your answer Mark. Accept my apologies for bothering you with already fixed issues. I had not found the appropriate discussions in the archive. Olivier Mark Thomas wrote: Olivier Jaquemet wrote: So my questions are : - What could be done in Tomcat to fix this ? This has been discussed extensively on the dev list and there is a patch proposed. See the archives for details. - Is the security problem real if it is why do other application server vendors do not have this behavior ? Yes the problem is real. It won't affect every installation or every application but it does affect some. As for the other app servers: don't know - haven't tested them. - As it break older application and it is not compatible with IE 7 in some circumstances. Do you really think this modification should be kept that way ? No. Again, see the discussion in the archives. Mark - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r639046 - /tomcat/tc6.0.x/trunk/STATUS.txt
Remy Maucherat wrote: On Wed, 2008-03-19 at 18:17 -0600, Filip Hanik - Dev Lists wrote: Remy Maucherat wrote: On Wed, 2008-03-19 at 22:43 +, [EMAIL PROTECTED] wrote: +1 : remm, markt - -1 : fhanik - I tried this patch, and it doesn't work, it hangs. I also saw the bug report, informing about further issues that are still valid + +1 : fhanik - I don't think we should support mark/reset, we can supply a filter that does it at the app level by wrapping getReader with a reader that bufferes That's used by that stupid readLine feature, I didn't add it to have fun :( sure doesn't sound like fun :) but given what a complicated impl that would end up being, I'd remove the support for it all together and have a Filter that does it with extreme simplicity just configured in the global web.xml file. the benefits are many 1. we don't need to have that kind of code support in our buffers, since it proves to be very complicated 2. users still get the support if wanted (one can of course turn it off) 3. the actual implementation of mark,reset and readLine becomes very easy any thoughts on taking that approach? Since a BufferedReader is provided, I would say it's better to support mark/reset. If removing it anyway, readLine should be redone. If I got this right, and correct me if I don't, This could have been so much easier, instead of Tomcat's complex implementation of the buffered reader. basically, if Tomcat implemented was just java.io.Reader, no buffering, no readline, no nothing, then the whole problem would have been solved by this simple filter (and I believe this still will solve the problem, even with todays code) public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { HttpServletRequest req = (HttpServletRequest)request; HttpServletRequestWrapper wrapper = new HttpServletRequestWrapper(req) { BufferedReader reader = null; public BufferedReader getReader() throws IOException { if (reader==null) { BufferedReader bf = super.getReader(); reader = new BufferedReader(bf); } return reader; } }; chain.doFilter(req, response); } what this filter does, is it relies on the already implemented BufferedReader, no need to reinvent the wheel. The java.io.BufferedReader, already has this logic implemented, and it works, so CoyoteReader, will only be treated as a java.io.Reader, and we can bypass all the stuff in Tomcat's code base Filip - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
DO NOT REPLY [Bug 44494] Requests greater than 8k being truncated.
https://issues.apache.org/bugzilla/show_bug.cgi?id=44494 --- Comment #18 from Filip Hanik [EMAIL PROTECTED] 2008-03-21 10:24:06 PST --- I posted to the dev lists about this, basically, one can just take advantage of java.io.BufferedReader to do the caching public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { HttpServletRequest req = (HttpServletRequest)request; HttpServletRequestWrapper wrapper = new HttpServletRequestWrapper(req) { BufferedReader reader = null; public BufferedReader getReader() throws IOException { if (reader==null) { BufferedReader bf = super.getReader(); reader = new BufferedReader(bf); } return reader; } }; chain.doFilter(req, response); } -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r639046 - /tomcat/tc6.0.x/trunk/STATUS.txt
On Fri, 2008-03-21 at 11:25 -0600, Filip Hanik - Dev Lists wrote: what this filter does, is it relies on the already implemented BufferedReader, no need to reinvent the wheel. The java.io.BufferedReader, already has this logic implemented, and it works, so CoyoteReader, will only be treated as a java.io.Reader, and we can bypass all the stuff in Tomcat's code base Maybe this piece of code is hopelessly broken and should be removed. If it's not, I think some effort should be made towards improving it [as well as improving the character conversion code], as double buffering is not something nice and a BufferedReader is a relatively expensive object. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
DO NOT REPLY [Bug 44494] Requests greater than 8k being truncated.
https://issues.apache.org/bugzilla/show_bug.cgi?id=44494 Remy Maucherat [EMAIL PROTECTED] changed: What|Removed |Added Status|ASSIGNED|NEEDINFO --- Comment #19 from Remy Maucherat [EMAIL PROTECTED] 2008-03-21 11:59:45 PST --- Did you really need to get three emails posted on the mailing list to express this idea ? Personally, I would be interested to know exactly which cases are not working at the moment after applying: Index: java/org/apache/catalina/connector/InputBuffer.java === --- java/org/apache/catalina/connector/InputBuffer.java (revision 633279) +++ java/org/apache/catalina/connector/InputBuffer.java (working copy) @@ -355,7 +355,7 @@ } state = CHAR_STATE; -conv.convert(bb, cb, len); +conv.convert(bb, cb, bb.getLength()); bb.setOffset(bb.getEnd()); return cb.getLength(); This includes no mark usage and with marks (which can probably be tested using readLine).Could I get some test cases ? -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
svn commit: r639771 - /tomcat/trunk/conf/web.xml
Author: markt Date: Fri Mar 21 12:29:35 2008 New Revision: 639771 URL: http://svn.apache.org/viewvc?rev=639771view=rev Log: No longer need to rename jars for CGI or SSI. Modified: tomcat/trunk/conf/web.xml Modified: tomcat/trunk/conf/web.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/conf/web.xml?rev=639771r1=639770r2=639771view=diff == --- tomcat/trunk/conf/web.xml (original) +++ tomcat/trunk/conf/web.xml Fri Mar 21 12:29:35 2008 @@ -277,11 +277,6 @@ !-- -- !-- outputEncoding The encoding to use for the page that results -- !-- from the SSI processing. [UTF-8] -- - !-- -- - !-- -- - !-- IMPORTANT: To use the SSI servlet, you also need to rename the -- - !--$CATALINA_HOME/server/lib/servlets-ssi.renametojar file -- - !--to $CATALINA_HOME/server/lib/servlets-ssi.jar -- !-- servlet @@ -334,10 +329,6 @@ !-- -- !-- passShellEnvironment Should the shell environment variables (if-- !--any) be passed to the CGI script? [false] -- - !-- -- - !-- IMPORTANT: To use the CGI servlet, you also need to rename the -- - !--$CATALINA_HOME/server/lib/servlets-cgi.renametojar file -- - !--to $CATALINA_HOME/server/lib/servlets-cgi.jar -- !-- servlet @@ -438,11 +429,6 @@ !-- Should virtual paths be interpreted as -- !-- relative to the context root, instead of -- !-- the server root? (0=false, 1=true) [0]-- - !-- -- - !-- -- - !-- IMPORTANT: To use the SSI filter, you also need to rename the-- - !--$CATALINA_HOME/server/lib/servlets-ssi.renametojar file -- - !--to $CATALINA_HOME/server/lib/servlets-ssi.jar -- !-- filter - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
svn commit: r639772 - /tomcat/tc6.0.x/trunk/STATUS.txt
Author: markt Date: Fri Mar 21 12:32:03 2008 New Revision: 639772 URL: http://svn.apache.org/viewvc?rev=639772view=rev Log: Propose another patch 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=639772r1=639771r2=639772view=diff == --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Mar 21 12:32:03 2008 @@ -121,3 +121,8 @@ http://svn.apache.org/viewvc?rev=639198view=rev +1: markt -1: + +* Update comments re JAR renaming in web.xml + http://svn.apache.org/viewvc?rev=639771view=rev + +1: + -1: \ No newline at end of file - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r639046 - /tomcat/tc6.0.x/trunk/STATUS.txt
Remy Maucherat wrote: On Fri, 2008-03-21 at 11:25 -0600, Filip Hanik - Dev Lists wrote: what this filter does, is it relies on the already implemented BufferedReader, no need to reinvent the wheel. The java.io.BufferedReader, already has this logic implemented, and it works, so CoyoteReader, will only be treated as a java.io.Reader, and we can bypass all the stuff in Tomcat's code base Maybe this piece of code is hopelessly broken and should be removed. If it's not, I think some effort should be made towards improving it [as well as improving the character conversion code], as double buffering is not something nice and a BufferedReader is a relatively expensive object. I'd prefer to move forward with just the truncation fix so that we can move towards a release. Any other efforts can be postponed at this time. If the mark/reset bug in fact exists, there is an easy work around, and that is to use java.io.BufferedReader. whether the code is hopelessly broken or not, I don't have enough info to comment on. But I do think the code is too fragile to be mucked with in our 6.0.x branch, that can be done in trunk or sandbox. It's better for us to get a release out that takes care of the regressions that are in the releases we have now. Filip - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r639046 - /tomcat/tc6.0.x/trunk/STATUS.txt
On Fri, 2008-03-21 at 14:29 -0600, Filip Hanik - Dev Lists wrote: I'd prefer to move forward with just the truncation fix so that we can move towards a release. Any other efforts can be postponed at this time. If the mark/reset bug in fact exists, there is an easy work around, and that is to use java.io.BufferedReader. whether the code is hopelessly broken or not, I don't have enough info to comment on. But I do think the code is too fragile to be mucked with in our 6.0.x branch, that can be done in trunk or sandbox. It's better for us to get a release out that takes care of the regressions that are in the releases we have now. This code has been used for a very long time. Where is the regression ? (actually, where is the bug ? feel free to post test cases to bug 44494) Personally, I am in no hurry to proceed with a release. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
svn commit: r639842 - /tomcat/trunk/webapps/docs/jndi-datasource-examples-howto.xml
Author: markt Date: Fri Mar 21 14:08:36 2008 New Revision: 639842 URL: http://svn.apache.org/viewvc?rev=639842view=rev Log: maxActive setting for unlimited changed in commons-pool 1.2 Modified: tomcat/trunk/webapps/docs/jndi-datasource-examples-howto.xml Modified: tomcat/trunk/webapps/docs/jndi-datasource-examples-howto.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/jndi-datasource-examples-howto.xml?rev=639842r1=639841r2=639842view=diff == --- tomcat/trunk/webapps/docs/jndi-datasource-examples-howto.xml (original) +++ tomcat/trunk/webapps/docs/jndi-datasource-examples-howto.xml Fri Mar 21 14:08:36 2008 @@ -219,7 +219,7 @@ lt;!-- maxActive: Maximum number of dB connections in pool. Make sure you configure your mysqld max_connections large enough to handle - all of your db connections. Set to 0 for no limit. + all of your db connections. Set to -1 for no limit. --gt; lt;!-- maxIdle: Maximum number of idle dB connections to retain in pool. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
svn commit: r639844 - /tomcat/tc6.0.x/trunk/STATUS.txt
Author: markt Date: Fri Mar 21 14:11:19 2008 New Revision: 639844 URL: http://svn.apache.org/viewvc?rev=639844view=rev Log: Propose doc fix 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=639844r1=639843r2=639844view=diff == --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Mar 21 14:11:19 2008 @@ -125,4 +125,9 @@ * Update comments re JAR renaming in web.xml http://svn.apache.org/viewvc?rev=639771view=rev +1: - -1: \ No newline at end of file + -1: + +* Fix docs re use of maxActive + http://svn.apache.org/viewvc?rev=639842view=rev + +1: + -1: - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
svn commit: r639845 - /tomcat/current/tc5.5.x/STATUS.txt
Author: markt Date: Fri Mar 21 14:11:35 2008 New Revision: 639845 URL: http://svn.apache.org/viewvc?rev=639845view=rev Log: Propose doc fix Modified: tomcat/current/tc5.5.x/STATUS.txt Modified: tomcat/current/tc5.5.x/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/current/tc5.5.x/STATUS.txt?rev=639845r1=639844r2=639845view=diff == --- tomcat/current/tc5.5.x/STATUS.txt (original) +++ tomcat/current/tc5.5.x/STATUS.txt Fri Mar 21 14:11:35 2008 @@ -74,3 +74,8 @@ http://people.apache.org/~markt/patches/2008-03-10-bug44556.patch +1: markt, fhanik -1: + +* Fix docs re use of maxActive + http://svn.apache.org/viewvc?rev=639842view=rev + +1: + -1: - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
svn commit: r639846 - /tomcat/current/tc4.1.x/STATUS.txt
Author: markt Date: Fri Mar 21 14:11:46 2008 New Revision: 639846 URL: http://svn.apache.org/viewvc?rev=639846view=rev Log: Propose doc fix Modified: tomcat/current/tc4.1.x/STATUS.txt Modified: tomcat/current/tc4.1.x/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/current/tc4.1.x/STATUS.txt?rev=639846r1=639845r2=639846view=diff == --- tomcat/current/tc4.1.x/STATUS.txt (original) +++ tomcat/current/tc4.1.x/STATUS.txt Fri Mar 21 14:11:46 2008 @@ -36,3 +36,8 @@ http://people.apache.org/~markt/patches/2008-03-10-bug41217-tc4.patch +1: markt -1: + +* Fix docs re use of maxActive + http://svn.apache.org/viewvc?rev=639842view=rev + +1: + -1: - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r639046 - /tomcat/tc6.0.x/trunk/STATUS.txt
Remy Maucherat wrote: On Fri, 2008-03-21 at 14:29 -0600, Filip Hanik - Dev Lists wrote: I'd prefer to move forward with just the truncation fix so that we can move towards a release. Any other efforts can be postponed at this time. If the mark/reset bug in fact exists, there is an easy work around, and that is to use java.io.BufferedReader. whether the code is hopelessly broken or not, I don't have enough info to comment on. But I do think the code is too fragile to be mucked with in our 6.0.x branch, that can be done in trunk or sandbox. It's better for us to get a release out that takes care of the regressions that are in the releases we have now. This code has been used for a very long time. Where is the regression ? (actually, where is the bug ? feel free to post test cases to bug 44494) regression, the fact that you can't read more than 8k when using ServletRequest.getReader Personally, I am in no hurry to proceed with a release. the regression mentioned above, and providing backwards compatibility with invalid v0 cookies IMO is enough to push another release Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r639046 - /tomcat/tc6.0.x/trunk/STATUS.txt
On Fri, 2008-03-21 at 15:19 -0600, Filip Hanik - Dev Lists wrote: Remy Maucherat wrote: regression, the fact that you can't read more than 8k when using ServletRequest.getReader I proposed a patch for that. If it doesn't fix it with the patch I proposed applied, I would need some test cases on bug 44494 to look at the problems. Personally, I am in no hurry to proceed with a release. the regression mentioned above, and providing backwards compatibility with invalid v0 cookies IMO is enough to push another release There are no agreed upon fixes for either problems, so I have no idea what could be released unfortunately. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r639046 - /tomcat/tc6.0.x/trunk/STATUS.txt
Remy Maucherat wrote: On Fri, 2008-03-21 at 15:19 -0600, Filip Hanik - Dev Lists wrote: Remy Maucherat wrote: regression, the fact that you can't read more than 8k when using ServletRequest.getReader I proposed a patch for that. If it doesn't fix it with the patch I proposed applied, I would need some test cases on bug 44494 to look at the problems. Personally, I am in no hurry to proceed with a release. the regression mentioned above, and providing backwards compatibility with invalid v0 cookies IMO is enough to push another release There are no agreed upon fixes for either problems, so I have no idea what could be released unfortunately. There are 3 +1's for the 8k issue (your proposed patch) and the cookies issue has 2. The cookies patch is short and you haven't voted yet so if you could find the time to review the cookies patch we could get to agreed fixes for both issues quite quickly. Mark - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r639046 - /tomcat/tc6.0.x/trunk/STATUS.txt
Remy Maucherat wrote: On Fri, 2008-03-21 at 15:19 -0600, Filip Hanik - Dev Lists wrote: Remy Maucherat wrote: regression, the fact that you can't read more than 8k when using ServletRequest.getReader I proposed a patch for that. If it doesn't fix it with the patch I proposed applied, I would need some test cases on bug 44494 to look at the problems. yes, and your proposed patch has 3 +1 votes. The second part of the bug, should be in a separate bug report if you ask me, and not relevant to this one. Personally, I am in no hurry to proceed with a release. the regression mentioned above, and providing backwards compatibility with invalid v0 cookies IMO is enough to push another release There are no agreed upon fixes for either problems, so I have no idea what could be released unfortunately. There is a cookie patch proposed, it follows the following behavior 1. If a cookie is added with version0 2. and that cookie contains illegal characters 3. then we switch the cookie to v1 most of this is already in SVN, the latest patch simply changes so that we only do a version switch based on characters in the value of the cookie. after that, path/domain etc is parsed using whatever version the cookie has at that point filip - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
svn commit: r639891 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/catalina/connector/InputBuffer.java webapps/docs/changelog.xml
Author: remm Date: Fri Mar 21 15:22:56 2008 New Revision: 639891 URL: http://svn.apache.org/viewvc?rev=639891view=rev Log: - 44494: patch for 8KB character input. - Votes. Modified: tomcat/tc6.0.x/trunk/STATUS.txt tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/InputBuffer.java tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Modified: tomcat/tc6.0.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=639891r1=639890r2=639891view=diff == --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Mar 21 15:22:56 2008 @@ -55,50 +55,6 @@ +0: remm: do we really want to fix these sort of bugs ? -1: -* Revert back to original patch proposed for UTF8 parsing. - This also fixes the regression for 6.0.16 and 5.5.26 (and possibly 4.1.37) mentioned in - http://issues.apache.org/bugzilla/show_bug.cgi?id=44494 - - The original patch can be viewed - http://people.apache.org/~fhanik/tomcat/b2c/patch.txt - - http://svn.apache.org/viewvc?rev=568307view=rev - http://svn.apache.org/viewvc?rev=568605view=rev - - +1: fhanik - -0: billbarker The original patch s*cks bigtime. I agree with remy (on list) that this issue should have - involved a discussion on list on how to fix, rather than just proposing a 5 sec fix. - -1: markt This patch removes a method from a public API. With that issue fixed -I would vote -0 until I can look at this further as explained in -http://marc.info/?l=tomcat-devm=120457882215329w=2 - -0: remm The newer code looks cleaner to me - -* As an alternate fix to 44494, I propose this patch (bb.getLength() is equal to the - value used as the result of available in the other proposed patch, and both - patches are thus using the same limit for reading data): - Index: java/org/apache/catalina/connector/InputBuffer.java -=== java/org/apache/catalina/connector/InputBuffer.java(revision 633279) -+++ java/org/apache/catalina/connector/InputBuffer.java(working copy) -@@ -355,7 +355,7 @@ - } - - state = CHAR_STATE; --conv.convert(bb, cb, len); -+conv.convert(bb, cb, bb.getLength()); - bb.setOffset(bb.getEnd()); - - return cb.getLength(); - - +1 : markt - +1 : fhanik - I don't think we should support mark/reset, we can supply a filter that does it at the app level by wrapping getReader with a reader that bufferes - +1 : remm: Along with the cookie fix, I would like to get a better review of all issues - before the next release (this fix as far as my testing goes improves things, though, so I think - it should go in for now), in particular marks (used by CoyoteReader.readLine, and which should - continue to be supported since the BufferedReader type returned by the Servlet API supports - marking) - -1 : - * Cookie completion. Only do the autoswitch on the value of the cookie, quote domain/path same way as we did before 258c258 version = maybeQuote2(version, buf, value); @@ -114,12 +70,12 @@ --- } else if (allowVersionSwitch (!STRICT_SERVLET_COMPLIANCE) version==0 !isToken2(value)) { - +1: fhanik, markt + +1: fhanik, markt, remm -1: * Remove reference to admin application http://svn.apache.org/viewvc?rev=639198view=rev - +1: markt + +1: markt, remm -1: * Update comments re JAR renaming in web.xml Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/InputBuffer.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/InputBuffer.java?rev=639891r1=639890r2=639891view=diff == --- tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/InputBuffer.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/InputBuffer.java Fri Mar 21 15:22:56 2008 @@ -355,7 +355,7 @@ } state = CHAR_STATE; -conv.convert(bb, cb, len); +conv.convert(bb, cb, bb.getLength()); bb.setOffset(bb.getEnd()); return cb.getLength(); Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=639891r1=639890r2=639891view=diff == --- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Fri Mar 21 15:22:56 2008 @@ -74,6 +74,9 @@ bug44558/bug: Improve error message so address is included if binding fails. /fix + fix +bug44494/bug: Character input limited to 8KB. (remm) + /fix /changelog /subsection subsection name=Jasper
Re: svn commit: r639046 - /tomcat/tc6.0.x/trunk/STATUS.txt
On Fri, 2008-03-21 at 21:44 +, Mark Thomas wrote: The cookies patch is short and you haven't voted yet so if you could find the time to review the cookies patch we could get to agreed fixes for both issues quite quickly. I did get lost somewhere between the new new patch and the new new new patch ;) Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
svn commit: r639909 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/tomcat/util/http/ServerCookie.java
Author: fhanik Date: Fri Mar 21 16:29:17 2008 New Revision: 639909 URL: http://svn.apache.org/viewvc?rev=639909view=rev Log: only make version switch on cookie value Modified: tomcat/tc6.0.x/trunk/STATUS.txt tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/ServerCookie.java Modified: tomcat/tc6.0.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=639909r1=639908r2=639909view=diff == --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Mar 21 16:29:17 2008 @@ -55,24 +55,6 @@ +0: remm: do we really want to fix these sort of bugs ? -1: -* Cookie completion. Only do the autoswitch on the value of the cookie, quote domain/path same way as we did before -258c258 - version = maybeQuote2(version, buf, value); - version = maybeQuote2(version, buf, value,true); -339a340,343 - return maybeQuote2(version,buf,value,false); - } - - public static int maybeQuote2 (int version, StringBuffer buf, String value, boolean allowVersionSwitch) { -348c352 - } else if ((!STRICT_SERVLET_COMPLIANCE) version==0 !isToken2(value)) { - } else if (allowVersionSwitch (!STRICT_SERVLET_COMPLIANCE) version==0 !isToken2(value)) { - - +1: fhanik, markt, remm - -1: - * Remove reference to admin application http://svn.apache.org/viewvc?rev=639198view=rev +1: markt, remm Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/ServerCookie.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/ServerCookie.java?rev=639909r1=639908r2=639909view=diff == --- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/ServerCookie.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/ServerCookie.java Fri Mar 21 16:29:17 2008 @@ -255,7 +255,7 @@ buf.append(=); // Servlet implementation does not check anything else -version = maybeQuote2(version, buf, value); +version = maybeQuote2(version, buf, value,true); // Add version 1 specific information if (version == 1) { @@ -337,6 +337,10 @@ * @param value */ public static int maybeQuote2 (int version, StringBuffer buf, String value) { +return maybeQuote2(version,buf,value,false); +} + +public static int maybeQuote2 (int version, StringBuffer buf, String value, boolean allowVersionSwitch) { if (value==null || value.length()==0) { buf.append(\\); }else if (containsCTL(value,version)) @@ -345,7 +349,7 @@ buf.append(''); buf.append(escapeDoubleQuotes(value,1,value.length()-1)); buf.append(''); -} else if ((!STRICT_SERVLET_COMPLIANCE) version==0 !isToken2(value)) { +} else if (allowVersionSwitch (!STRICT_SERVLET_COMPLIANCE) version==0 !isToken2(value)) { buf.append(''); buf.append(escapeDoubleQuotes(value,0,value.length())); buf.append(''); - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
svn commit: r639910 - /tomcat/tc6.0.x/trunk/STATUS.txt
Author: fhanik Date: Fri Mar 21 16:30:48 2008 New Revision: 639910 URL: http://svn.apache.org/viewvc?rev=639910view=rev Log: votes 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=639910r1=639909r2=639910view=diff == --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Mar 21 16:30:48 2008 @@ -57,15 +57,15 @@ * Remove reference to admin application http://svn.apache.org/viewvc?rev=639198view=rev - +1: markt, remm + +1: markt, remm, fhanik -1: * Update comments re JAR renaming in web.xml http://svn.apache.org/viewvc?rev=639771view=rev - +1: + +1: fhanik -1: * Fix docs re use of maxActive http://svn.apache.org/viewvc?rev=639842view=rev - +1: + +1: fhanik -1: - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
svn commit: r639919 - /tomcat/tc6.0.x/trunk/STATUS.txt
Author: markt Date: Fri Mar 21 16:49:17 2008 New Revision: 639919 URL: http://svn.apache.org/viewvc?rev=639919view=rev Log: Must remember to vote for my own proposals. 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=639919r1=639918r2=639919view=diff == --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Mar 21 16:49:17 2008 @@ -62,10 +62,10 @@ * Update comments re JAR renaming in web.xml http://svn.apache.org/viewvc?rev=639771view=rev - +1: fhanik + +1: markt, fhanik -1: * Fix docs re use of maxActive http://svn.apache.org/viewvc?rev=639842view=rev - +1: fhanik + +1: markt, fhanik -1: - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
svn commit: r639920 - in /tomcat/tc6.0.x/trunk: STATUS.txt webapps/ROOT/admin/
Author: markt Date: Fri Mar 21 16:50:39 2008 New Revision: 639920 URL: http://svn.apache.org/viewvc?rev=639920view=rev Log: Remove reference to admin application. Removed: tomcat/tc6.0.x/trunk/webapps/ROOT/admin/ 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=639920r1=639919r2=639920view=diff == --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Mar 21 16:50:39 2008 @@ -55,11 +55,6 @@ +0: remm: do we really want to fix these sort of bugs ? -1: -* Remove reference to admin application - http://svn.apache.org/viewvc?rev=639198view=rev - +1: markt, remm, fhanik - -1: - * Update comments re JAR renaming in web.xml http://svn.apache.org/viewvc?rev=639771view=rev +1: markt, fhanik - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]