[GitHub] [tomcat] rmaucher commented on pull request #598: Use available constants
rmaucher commented on PR #598: URL: https://github.com/apache/tomcat/pull/598#issuecomment-1465711314 The thing to look for is to not add any dependencies to the bootstrap, but it looks ok here (I think). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] ChristopherSchultz commented on pull request #596: Use SELECT...FOR UPDATE to update session data in the database instead of DELETE, INSERT.
ChristopherSchultz commented on PR #596: URL: https://github.com/apache/tomcat/pull/596#issuecomment-1466223964 > I have a question that why we don't add a real **primary key**(auto-increment) to solve the problem that primary key constraint violation when insert data to database simultaneously? The problem is that there is a window of opportunity between the existing `DELETE` and `INSERT` where the `session_id` column (which is `UNIQUE` or equivalent) can be `INSERT`ed by another thread. Changing the definition of the table to include a separate PK and removing the `UNIQUE` constraint from the `session_id` column is one way to solve this issue, but it would break any existing installation which is _not_ using that kind of table definition. I would definitely *not* want to do that for a point-release, and my goal here is to back-port this all the way back to 8.5 at this point. I think some larger changes could be made separately from this current effort, and restricted to maybe Tomcat 11.0.x and later. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] ChristopherSchultz commented on pull request #596: Use SELECT...FOR UPDATE to update session data in the database instead of DELETE, INSERT.
ChristopherSchultz commented on PR #596: URL: https://github.com/apache/tomcat/pull/596#issuecomment-1466230691 > I would love to see an interface that goes a step further and allows for NoSQL implementations as well. For example, Redis is an excellent option for a data store IMO. This is the whole point of the `Store` interface. If you want to implement your own strategy, you can implement it however you want. I believe there are some Redis- and Memcached-based `Store` implementations already available from third-parties. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] ChristopherSchultz commented on pull request #596: Use SELECT...FOR UPDATE to update session data in the database instead of DELETE, INSERT.
ChristopherSchultz commented on PR #596: URL: https://github.com/apache/tomcat/pull/596#issuecomment-1466235515 > Punt the session update operation to a callback or interface, have one or two sensible defaults (like for the major databases and specific table configuration) and have a way for the user to provide their own implementation? If you are going to provide your own implementation, you could just subclass DataSourceStore and change the behavior of the `store` (or `load`, etc.) methods. This does bring up a good point: rather than having a new configuration option, I could simply have a subclass of DataSourceStore which overrides the `store` method to do the `SELECT...FOR UPDATE`. Then there is a much smaller change to the existing code, which is beneficial for stability. In fact, I think no existing code needs to change; everything is in the new class, including this optional new _primary key column_ option. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] isapir commented on pull request #596: Use SELECT...FOR UPDATE to update session data in the database instead of DELETE, INSERT.
isapir commented on PR #596: URL: https://github.com/apache/tomcat/pull/596#issuecomment-1466395493 > This does bring up a good point: rather than having a new configuration option, I could simply have a subclass of DataSourceStore which overrides the store method to do the SELECT...FOR UPDATE. Then there is a much smaller change to the existing code, which is beneficial for stability. In fact, I think no existing code needs to change; everything is in the new class, including this optional new primary key column option. That's a great idea. It can also serve as a model for others who might want to extend the DataSourceStore. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 66196] HTTP/1 connector doesn't blow-up when HTTP header contains non-ASCII characters
https://bz.apache.org/bugzilla/show_bug.cgi?id=66196 --- Comment #11 from gabriel.holl...@appian.com --- Looking over the thread here, it sounded like the path forward was to log, and potentially drop the header for Http1.1 However that same change wasnt done for the AJP Processor, causing invalid headers to suddenly break upon tomcat upgrade (discover in our 8.5.x usage) java.lang.IllegalArgumentException: The Unicode character [–] at code point [8,211] cannot be encoded as it is outside the permitted range of 0 to 255 at org.apache.tomcat.util.buf.MessageBytes.toBytesSimple(MessageBytes.java:290) at org.apache.tomcat.util.buf.MessageBytes.toBytes(MessageBytes.java:261) at org.apache.coyote.ajp.AjpMessage.appendBytes(AjpMessage.java:172) at org.apache.coyote.ajp.AjpProcessor.prepareResponse(AjpProcessor.java:1121) at org.apache.coyote.ajp.AjpProcessor$SocketOutputBuffer.doWrite(AjpProcessor.java:1511) at org.apache.coyote.Response.doWrite(Response.java:602) at org.apache.catalina.connector.OutputBuffer.realWriteBytes(OutputBuffer.java:356) at org.apache.catalina.connector.OutputBuffer.flushByteBuffer(OutputBuffer.java:846) at org.apache.catalina.connector.OutputBuffer.realWriteChars(OutputBuffer.java:470) at org.apache.catalina.connector.OutputBuffer.flushCharBuffer(OutputBuffer.java:851) at org.apache.catalina.connector.OutputBuffer.close(OutputBuffer.java:250) at org.apache.catalina.connector.CoyoteWriter.close(CoyoteWriter.java:107) Is there any chance the same handling for HTTP 1.1 could apply to AJP? -- 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 66196] HTTP/1 connector doesn't blow-up when HTTP header contains non-ASCII characters
https://bz.apache.org/bugzilla/show_bug.cgi?id=66196 Mark Thomas changed: What|Removed |Added Resolution|FIXED |--- Status|RESOLVED|REOPENED --- Comment #12 from Mark Thomas --- Re-opening for visibility -- 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 66524] resource cache eviction is MRU not LRU
https://bz.apache.org/bugzilla/show_bug.cgi?id=66524 --- Comment #3 from Christopher Schultz --- (In reply to Steven Kearns from comment #2) > The field CachedResource.nextCheck is what is being sorted, it is also a > proxy for lastUsedTime. Yup. I got the reversal reversed in my head when reading it as well. We want to expire the lowest value for "nextCheck" because it is the same as the one with the oldest "lastCheck", and therefore the least-recently-used. I agree with your analysis. I'd like a "second" from another committer before moving forward with the 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
[Bug 66524] resource cache eviction is MRU not LRU
https://bz.apache.org/bugzilla/show_bug.cgi?id=66524 --- Comment #4 from Mark Thomas --- +1 -- 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
