[GitHub] tomcat issue #137: Add HTML lang="en"
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/137 > 404/403/401.jsp pages in manager and host-manager may benefit from localization, and in that case the lang attribute has to be calculated dynamically to reflect the actual language being used. Though currently those pages are hard-coded with some English text. When introducing i18n for things like this, it's important to know which language was actually chosen when requesting, for example, some unsupported language. The `StringManager` only uses one locale, but it never checks to see which locale it actually got. Client code can't request a string from a particular locale... it can only ask the StringManager which locale it's actually serving. Java's `ResourceBundle` is actually insufficient for implementing real i18n unless every single string is always translated in each resource bundle into each language for which a bundle file exists. When you have non-100% coverage, it's not possible to use just `ResourceBundle` to do the job. Instead, you need a hierarchical system based upon `ResourceBundle` and a few other things. It gets messy. If we just want to use whatever `StringManager` loads on startup, or instantiate a new `StringManager` for each requested-locale and hope that we have 100% string-coverage, we can use that and just put `
[GitHub] tomcat issue #110: Debug log for pool's db properties
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/110 So you were providing a `Properties` object that was not loaded from a file, and had non-string values in it? Your patch would not identify that problem. Non-string values should probably be allowed within tomcat-pool. Care to do a PR for that as well? --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #110: Debug log for pool's db properties
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/110 What problem does this solve? What property file do you have that contains non-strings? --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #103: Use initSQL instead of Validator on connection initializa...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/103 Honestly, I would expect `initSQL` to be executed regardless of any other configuration (e.g. Validator present or not). --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #107: underlying connection closed unexpectedly, so we must des...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/107 Is this "just" a mild performance optimization? It looks like more code where no more code is needed. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #108: Improve undeployment in parallel deployment scenario
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/108 I'm starting to lean toward requiring this new feature to require a configuration option to enable it, and have it default to `false`. My justification is that it represents a breaking behavioral change that is *just slightly different* than previous behavior as to be unnoticeable until it starts behaving completely unexpectedly. Can you please add a configuration option for `` to enable or disable this? Simply swap-out the `compareTo` implementation depending upon the value of that setting. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #108: Improve undeployment in parallel deployment scenario
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/108 The performance of this strategy can be significantly improved by converting the "version number" into a value that can more easily be compared against other values. This can be done in several ways, some building on others (i.e. smaller changes) and some with significant changes. The most obvious performance optimization is to cache the compiled regex `Pattern` objects: you will be doing a lot of tokenizing on `/\./`, so compile it once and use it many times. The second most obvious performance optimization is to stop using regular expressions entirely. When searching for a single character, it's much faster to write your own tokenizer because regular expressions inherently have more overhead (because they are so flexible). The code is not as straightforward (and I'm happy to see that you provided an easy-to-follow patch, here) but in this case, speed is in fact important as Mark notes: this code will be executed for every request whose URL might match the context. The third most obvious performance improvement would be to store the tokenized version number as an array and, in `compareTo` simply compare the individual elements of the arrays in each object. Then you don't have to tokenize during each compare. Finally, instead of re-computing the "version" difference every time, a representative value could be built for the version number that can be quickly compared against another value. For example, let's take the versions `1.0.9` and `1.0.10` for example. One strategy would be to normalize each part of the version number to have some kind of "maximum number of digits." Let's pick 4-digits and see how to do this: 1. Tokenize `1.0.9` into [1, 0, 9] 2. Convert to string with 4-digits for each version component: `00010009` 3. Tokenize `1.0.10` into [1, 0, 10] 4. Convert to string with 4-digits for each version component: `00010010` 5. Compare the strings alphabetically (`String.compareTo`) Now, simply more steps 1-2 into the constructor of the `ContextName` class and then `ContextName.compareTo` becomes a simple comparison of the normalized version number. One can also do this with integral values instead of `Strings` and the comparison becomes even faster, but you run the risk of running out of digits if the padding is too wide (e.g. 4-digit padding with a version number like `1.0.9.1` yields `1000200030001` which is `>` `Integer.MAX_VALUE` so you'd have to switch to a `long` value. Comparing `long` values is faster than comparing `String` values, but at some point you run out of digits in a `long` value as well. Anyway, there is lots of room for improvement, here, and I think it *does* make sense to support "traditional" version numbering. Note that this will represent a backward-*in*compatible change, and those relying upon the current alphabetical-ordering will need to adjust their expectations. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #96: Remove PUT and DELETE methods from an OPTIONS request if r...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/96 WRT WebDav, [RFC2618](https://tools.ietf.org/html/rfc2518) says that WebDav is an extension to HTTP/1.1 so I think we are always okay using 405 from `WebDavServlet`. We should, however, take care to adhere to the spec as much as possible, so we should e.g. include an `Allow` header in the response. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #96: Remove PUT and DELETE methods from an OPTIONS request if r...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/96 I think 405 is more appropriate when the method is in fact not allowed (readOnly). As for POST, there is nothing "writey" about POST, whereas PUT and DELETE have definite "write" semantics. The fact that POST refers to GET is an irrelevant implementation detail. Plenty of (non-WebDAV) GET requests cause irreversible changes on the server (due to the implementation of a particular web application, not a vanilla web server itself) even when the official standard suggests that GET should be idempotent and read-only. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #96: Remove PUT and DELETE methods from an OPTIONS request if r...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/96 Patch looks good to me. Regarding your questions: > Why is a POST allowed when readOnly is true? Probably because the DefaultServlet just delegates POST -> GET, but ... > Why does a POST call GET internally? Good question. I suspect because that's what httpd does when making a POST request to a static resource. That code has not changed since 2006 (the initial import) so at this point, it's "legacy" and "backward-compatible" and extremely unlikely to change. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #76: added SessionInitializerFilter
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/76 @markt-asf re: `Filter` versus configuration This could be something that we add now just as a `Filter` but in the future add a configuration option that just automatically attaches the `Filter` -- just like how using `` in `WEB-INF/web.xml` auto-attaches the appropriate `Valve` to the valve chain. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #79: remove placeholders from introduction doc
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/79 References: https://apacheconna2015.sched.com/event/2P6d/rtfm-write-a-better-fm-rich-bowen-apache-software-foundation http://feathercast.apache.org/podcasts/ApacheConNA2015/Monday/Community/Monday_Community_04%20-%20Rich%20Bowen%20-%20RTFM%20-%20Write%20a%20Better%20FM.mp3 http://drbacchus.com/rtfm-write-a-better-fm-recording/ --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #79: remove placeholders from introduction doc
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/79 A few years ago, Rich Bowen gave a talk @ ApacheCon about documentation and communities. He used Python as an example of a community where the Monty Python jokes are so pervasive, it appears that they have become the focus of the documentation instead of the actual subject (the code, etc.). He suggested that humor, while intended to be fun, lighthearted, etc. often comes across as mean-spirited or condescending, and/or introduces confusion especially to those who do not understand (a) the language (e.g. English), (b) that they are in fact jokes or (c) the content of the jokes (e.g. unfamiliar with Monty Python). Basically, this kind of thing doesn't have a place in high-quality documentation. This example is relatively benign, but there are probably other places where the humor is at least distracting if not worse. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 Not quite yet. Let's see if we can get it closed automatically through the "proper" channels. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 I'm not sure why the commit message hasn't closed this issue. Any thoughts on why that might be? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 Committed to tomcat-trunk in ASF svn r1799462. Back-ports to follow. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 Thanks @Igal for your patience with this PR review. I know we've been a bit of a pain. But this work will make the contribution that much more useful. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 I'm waiting on a final review from @KeiichiFujino before I commit. In the meantime there are 2 `@return` javadoc annotations with no value. Could you complete those? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 @isapir FYI `1 << bit` is the same as `(int)Math.pow(2, bit)` and probably slightly more efficient. Better yet, it's less code to read. No particular reason to change the patch. I'm already happy with what we've got. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 @KeiichiFujino Okay, so we'd have a synthetic mbean attribute that we can re-construct from the int value we store internally? Or, do you think we should save the string that was used in configuration? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 @KeiichiFujino Is this patch acceptable as-is, with additional patches for your items listed above? I think it's okay to add this feature only to channelSendOptions as a first commit. Adding mapSendOptions and JMX support can come later. Okay? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #56: Convert Cluster Manager human-readable SendOptions names t...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 How would you like your name to appear in the changelog? "Patch provided by Igal Sapir"? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #:
Github user ChristopherSchultz commented on the pull request: https://github.com/apache/tomcat/commit/d91d7a802b67ab049b33df9db017ef31d3247450#commitcomment-22378483 @powerYao Because in the distant past, someone decided that having those extra parentheses was good coding style. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #56: Convert Cluster Manager human-readable SendOptions names t...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 @isapir I appreciate your core patch as an initial effort. It looks like it will work. In order for it to be a good patch, it needs to be expanded, so a larger patch is appropriate. Did you actually test it to see if it would give you the desired channel options? A small unit test wouldn't hurt, as well (just testing setChannelSendOptions(String) -> getChannelSendOptions:int would be fine). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #56: Convert Cluster Manager human-readable SendOptions names t...
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 A few comments: 1. I think the patch should include some TRACE-level logging, especially when catching the NumberFormatException. 2. If there is an unrecognized option string, it should at least result in a WARN message emitted to the log. I might even lobby for an exception to be thrown. 3. There are options that seem to be in conflict with each other, if only in terminology: e.g. "async" and "sync" can be specified together, which would read like a bug in the config. 4. Splitting on a comma should include optional whitespace e.g. "\\s,*\\s*" as the pattern. 5. The patch needs to include documentation for the string-based configuration options (webapps/docs/cluster-howto.html). 6. The javadoc in the patch needs to include the list of acceptable string option-names. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #51: #41 Check and update session record if one already exists.
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/51 Might this be better written as `SELECT ... FOR UPDATE` with either `updateRow` or `ResultSet.insert` if there are no results from the `SELECT`? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #41: Check and update session record if one already exists.
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/41 You should use SELECT ... FOR UPDATE instead of SELECT / UPDATE. Much less code and transactionally-safe. Also, if you use SELECT ... FOR UPDATE, you can use ResultSet.moveToInsertRow to insert a record if necessary. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #39: Provide the correct port when using proxy
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/39 This looks like a no-op change. Does the CONNECT verb not imply a default port number when none is present? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
