[GitHub] tomcat pull request: Fixed UnsupportedOperationException when rele...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/23 --- 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: Support of Digest authentication in the JNDIR...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/24 --- 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 markt-asf commented on the pull request: https://github.com/apache/tomcat/commit/548d60449a40248e9b7fb2a8103a5e4419d6c290#commitcomment-14730299 The patch is unnecessary. The Tribes StringManager does not have to support multiple locales whereas the main StringManager (o.a.t.u.res) does. There are multiple StringManager implementations to avoid unwanted dependencies. --- 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: Change response character encoding
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/25 --- 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 #115: Setting Timezone to GMT for Expires Header as per RFC1123
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/115 Thanks for the PR. We have fixed this but with a simpler approach that makes use of `ConcurrentDateFormat.formatRfc1123()` --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #115: Setting Timezone to GMT for Expires Header as per ...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/115 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #118: Fix typos detected by github.com/client9/misspell
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/118 Thanks for this. Working through a review of the patch now. Note that there are some files that we can't change such as the Oracle provided XSDs. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #118: Fix typos detected by github.com/client9/misspell
Github user markt-asf commented on a diff in the pull request: https://github.com/apache/tomcat/pull/118#discussion_r209290720 --- Diff: java/org/apache/tomcat/dbcp/dbcp2/managed/ManagedConnection.java --- @@ -1,4 +1,4 @@ -/** +vi /** --- End diff -- No worries. The git repo is only a mirror so I have to manually apply to the patch to svn anyway. I'd already found this one and fixed it. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #118: Fix typos detected by github.com/client9/misspell
Github user markt-asf commented on a diff in the pull request: https://github.com/apache/tomcat/pull/118#discussion_r209292288 --- Diff: java/org/apache/catalina/ha/session/SessionMessageImpl.java --- @@ -144,7 +144,7 @@ public String getEventTypeString() case EVT_GET_ALL_SESSIONS : return "SESSION-GET-ALL"; case EVT_SESSION_DELTA : return "SESSION-DELTA"; case EVT_ALL_SESSION_DATA : return "ALL-SESSION-DATA"; -case EVT_ALL_SESSION_TRANSFERCOMPLETE : return "SESSION-STATE-TRANSFERED"; +case EVT_ALL_SESSION_TRANSFERCOMPLETE : return "SESSION-STATE-TRANSFERRED"; --- End diff -- This one is fine. It is only used for logging. And the similar issue in DeltaManager should be OK too. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #117: Enhance the CATALINA_BASE documentation
Github user markt-asf commented on a diff in the pull request: https://github.com/apache/tomcat/pull/117#discussion_r209306777 --- Diff: webapps/docs/introduction.xml --- @@ -89,6 +80,122 @@ same as $CATALINA_HOME. + + Throughout the documentation, there are references to the two following +properties: + + +CATALINA_HOME: Represents the root of your Tomcat +installation, for example /home/tomcat/apache-tomcat-9.0.10 +or C:\Program Files\apache-tomcat-9.0.10. + + +CATALINA_BASE: Represents the root of a runtime +configuration of a specific Tomcat instance. If you want to have +multiple Tomcat instances on one machine, use the CATALINA_BASE +property. + + + + +If you set the properties to different locations, the CATALINA_HOME location +contains static sources, such as .jar files, or binary files. +The CATALINA_BASE location contains configuration files, log files, deployed +applications, and other runtime requirements. + + + + By default, CATALINA_HOME and CATALINA_BASE point to the same directory. + Set CATALINA_BASE manually when you require running multiple Tomcat + instances on one machine. Doing so provides the following benefits: + + + +Easier management of upgrading to a newer version of Tomcat. Because all +instances with single CATALINA_HOME location share one set of +.jar files and binary files, you can easily upgrade the files +to newer version and have the change propagated to all Tomcat instances +using the same CATALIA_HOME directory. + + +Avoiding duplication of the same static .jar files. + + +The possibility to share certain settings, for example the setenv shell +or bat script file (depending on your operating system). + + + + + + Before you start using CATALINA_BASE, create the directory you want to use + as CATALINA_BASE for the particular Tomcat instance. At minimum, it must + contain: + +conf/server.xml +conf/web.xml + + That includes the conf directory. Otherwise, Tomcat may + fail to start. + + + Additionally, it may also contain the following: + --- End diff -- I think some slightly stronger wording is required here. Not having some of these directories is going to cause problems. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #117: Enhance the CATALINA_BASE documentation
Github user markt-asf commented on a diff in the pull request: https://github.com/apache/tomcat/pull/117#discussion_r209308073 --- Diff: webapps/docs/introduction.xml --- @@ -89,6 +80,122 @@ same as $CATALINA_HOME. + + Throughout the documentation, there are references to the two following +properties: + + +CATALINA_HOME: Represents the root of your Tomcat +installation, for example /home/tomcat/apache-tomcat-9.0.10 +or C:\Program Files\apache-tomcat-9.0.10. + + +CATALINA_BASE: Represents the root of a runtime +configuration of a specific Tomcat instance. If you want to have +multiple Tomcat instances on one machine, use the CATALINA_BASE +property. + + + + +If you set the properties to different locations, the CATALINA_HOME location +contains static sources, such as .jar files, or binary files. +The CATALINA_BASE location contains configuration files, log files, deployed +applications, and other runtime requirements. + + + + By default, CATALINA_HOME and CATALINA_BASE point to the same directory. + Set CATALINA_BASE manually when you require running multiple Tomcat + instances on one machine. Doing so provides the following benefits: + + + +Easier management of upgrading to a newer version of Tomcat. Because all +instances with single CATALINA_HOME location share one set of +.jar files and binary files, you can easily upgrade the files +to newer version and have the change propagated to all Tomcat instances +using the same CATALIA_HOME directory. + + +Avoiding duplication of the same static .jar files. + + +The possibility to share certain settings, for example the setenv shell +or bat script file (depending on your operating system). + + + + + + Before you start using CATALINA_BASE, create the directory you want to use + as CATALINA_BASE for the particular Tomcat instance. At minimum, it must + contain: + +conf/server.xml +conf/web.xml + + That includes the conf directory. Otherwise, Tomcat may + fail to start. + + + Additionally, it may also contain the following: + + + The bin directory with the setenv.sh, + setenv.bat, and tomcat-juli.jar files. + + + The lib directory with further resources to be added on + classpath. + + + The logs directory for instance-specific log files. + + + The webapps directory for automatically loaded web + applications. + + + The work directory that contains temporary working + directories for the deployed web applications. + + + The temp directory used by the JVM for temporary files. + + + + + We recommend you not to change the tomcat-juli.jar file. + However, in case you require your own logging implementation, you can + replace the tomcat-juli.jar file in a CATALINA_BASE location + for the specific Tomcat instance. + + + We also recommend you not to copy over any configuration file that is identical + for multiple Tomcat instances, with the exception of the mandatory files. + For example, copy over the conf/logging.properties only if + you require the particular Tomcat instance to have specific logging settings. + --- End diff -- That won't work. Tomcat looks in CATALINA_BASE for the logging configuration. You could change that in catalina.[sh|bat] but that starts to make the setup more complicated. Generally, ALL configuration files are only read form BASE and DO NOT fall back to HOME. The only exception in bin/setenv.sh and this onyl falls back to HOME for historical backwards compatibility reasons. I'd suggest updating this to say copy all of conf/* across. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #117: Enhance the CATALINA_BASE documentation
Github user markt-asf commented on a diff in the pull request: https://github.com/apache/tomcat/pull/117#discussion_r20982 --- Diff: webapps/docs/introduction.xml --- @@ -89,6 +80,122 @@ same as $CATALINA_HOME. + + Throughout the documentation, there are references to the two following +properties: + + +CATALINA_HOME: Represents the root of your Tomcat +installation, for example /home/tomcat/apache-tomcat-9.0.10 +or C:\Program Files\apache-tomcat-9.0.10. + + +CATALINA_BASE: Represents the root of a runtime +configuration of a specific Tomcat instance. If you want to have +multiple Tomcat instances on one machine, use the CATALINA_BASE +property. + + + + +If you set the properties to different locations, the CATALINA_HOME location +contains static sources, such as .jar files, or binary files. +The CATALINA_BASE location contains configuration files, log files, deployed +applications, and other runtime requirements. + + + + By default, CATALINA_HOME and CATALINA_BASE point to the same directory. + Set CATALINA_BASE manually when you require running multiple Tomcat + instances on one machine. Doing so provides the following benefits: + + + +Easier management of upgrading to a newer version of Tomcat. Because all +instances with single CATALINA_HOME location share one set of +.jar files and binary files, you can easily upgrade the files +to newer version and have the change propagated to all Tomcat instances +using the same CATALIA_HOME directory. + + +Avoiding duplication of the same static .jar files. + + +The possibility to share certain settings, for example the setenv shell +or bat script file (depending on your operating system). + + + + + + Before you start using CATALINA_BASE, create the directory you want to use + as CATALINA_BASE for the particular Tomcat instance. At minimum, it must + contain: + +conf/server.xml +conf/web.xml + + That includes the conf directory. Otherwise, Tomcat may + fail to start. + + + Additionally, it may also contain the following: + --- End diff -- Strictly speaking, I agree with you. My concern is that a novice user will read this and think they only need those files whereas most users will want at least logs, temp, webapps, work, most of conf/* and probably lib too. Just considering the work directory - if that is missing and Tomcat can't create one lots of stuff breaks, the most obvious of which is JSP compilation. What I think is required here is a change of emphasis. Maybe start with what is typically required (most / all of it) and then discuss what the bare minimum looks like. I'd also add notes to each dir about the relationship between HOME and BASE. ie for bin/setenv.sh Tomcat looks in BASE then HOME. For lib, Tomcat looks in BASE then HOME (configured in conf/catalina.propertis), for all other dirs Tomcat only looks in BASE (unless configured to use an absolute path that points elsewhere which could include HOME). --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #117: Enhance the CATALINA_BASE documentation
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/117 Looks great. Applied. Many thanks. (note: there was one s/HOME/BASE/ required in the webapps section) --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #120: fix -Djava.security.policy setting
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/120 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #120: fix -Djava.security.policy setting
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/120 The double = is deliberate. The OP needs to RTFM. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #122: Added a default value for ApplicationSessionCookieConfig#...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/122 Closing the PR as the requested change is not correct. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #122: Added a default value for ApplicationSessionCookie...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/122 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #122: Added a default value for ApplicationSessionCookieConfig#...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/122 There is nothing in the spec that says Tomcat is required to do that. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request: Honour server side precompressed format prefe...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/30 --- 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 #33: Clear up confusion about mbeans-descriptors.xml in docs; f...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/33 Rename the page. If someone complains, we can look at adding a redirect although we'll need to handle both Tomcat (for releases) and httpd (for the website). I'd prefer to have a single solution for both but I don;t see one at this point. --- 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 #33: Clear up confusion about mbeans-descriptors.xml in ...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/33 --- 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 #47: Make ParameterMap immutable using containment instead of i...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/47 Patch applied. Thanks. --- 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 #47: Make ParameterMap immutable using containment inste...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/47 --- 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 markt-asf commented on the pull request:
https://github.com/apache/tomcat/commit/d46012cb159c3384303eeceffc54eb78e6a62671#commitcomment-21553794
In java/org/apache/el/Messages.properties:
In java/org/apache/el/Messages.properties on line 32:
Let me take a closer look at this one. Generally, I think we need to move
away from using ''{n}'' and use [{n}] more consistently.
---
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 #50: update the download of objenesis
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/50 This change was made to trunk over 6 months ago. --- 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 #50: update the download of objenesis
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/50 --- 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 #52: Commit the added comment on Application.java
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/52 Spam --- 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 #52: Commit the added comment on Application.java
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/52 --- 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 #53: Commit the added comment on Application.java
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/53 Spam --- 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 #53: Commit the added comment on Application.java
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/53 --- 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: merge lastest update
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/26 --- 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: merge lastest update
Github user markt-asf commented on the pull request: https://github.com/apache/tomcat/pull/26#issuecomment-172488725 It appears this pull request was opened in error. --- 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: test
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/27 --- 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: test
Github user markt-asf commented on the pull request: https://github.com/apache/tomcat/pull/27#issuecomment-184110339 Spam. This idiot has been blocked. --- 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: Generalize the precompressed content serving ...
Github user markt-asf commented on the pull request: https://github.com/apache/tomcat/pull/28#issuecomment-188460191 Generally I like this patch. I do have a couple of comments: - I think it makes sense to make the change to a generic approach immediately. One way of doing this would be for the precompressed init param to take a comma separated list like ".gz,gzip,.br,br". It would need suitable sanity checking. - The preference for different encodings is set by the client, not the server. See RFC7231, section 5.3.4. Look at o.a.catalina.Request.parseLocales() for a way of handling q values. --- 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 #124: Allow empty 'Host' HTTP header
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/124 Nice catch with handling the port. The patch I had come up with missed that. I refactored populateHost() as it bugged me to set the server name one way (that might throw an IOE and break the request) only to immediately overwrite the result with an empty String. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #125: Provide port offset functionality (BZ-61171)
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/125 I've skimmed this and it looks good. I'm currently buried in TLS 1.3 stuff but wanted to let you know that the PR had been seen and that I hope to get it into the next 9.0.x release. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #126: The feature of the transfer rate control are added to the...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/126 -1 here too for much the same reason. A filter would work better for this. Ignoring the thread related concerns, I'd also point out that new features without a description of the use case or similar justification are very unlikely to be accepted. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #126: The feature of the transfer rate control are added...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/126 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #127: Add a fake attribute for source
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/127 Nice idea. I've applied a variation of this patch to Tomcat. The key changes were: - Only ignore source on Context elements - Ignore source on Context elements in server.xml and context.xml files --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #123: Checking the signing service response
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/123 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #123: Checking the signing service response
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/123 Thanks for reviewing this code and providing a patch. Unfortunately, we plan to shortly move away from our custom signing service and switch to the solution provided by DigiCert that integrates a command line tool with the signing service that we can then call from our build script. Therefore, we will not be applying this PR. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #125: Provide port offset functionality (BZ-61171)
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/125 I've now spent some time looking at this more closely. I like the idea of setting this once on the `Server` and then auto-magically setting this on the `Connector`s. As I reviewed this, I noticed that we are still setting the port in multiple places. That seems wrong to me as it creates the possibility of having inconsistent settings. I am leaning towards refactoring `port` in the `Connector` so it always delegates to the `Endpoint`. `portOffset` would then be handled the same way. I am a little concerned about having `portOffset` on the `Server` and the `Connector`. Again, there is scope there for the settings to become inconsistent. However, I don't see any easy way around that. Finally, I am still mulling over the extent to which the currently used port is exposed as `portWithOffset` and when `port` and `offset` are exposed separately. I'm leaning towards a wider use of `portWithOffset` and separate log messages that provide `portWithOffset`, `port` and `offset`. My current plan is to refactor port as describe above and then start to integrate this patch. I'm expecting to tweak a few things as I go and I'm still aiming for inclusion in 9.0.13. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #129: Fix typo in Spanish translation
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/129 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #129: Fix typo in Spanish translation
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/129 Thanks. Patch applied to 9.0.x, 8.5.x and 7.0.x for the next release of each. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #125: Provide port offset functionality (BZ-61171)
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/125 Patch has been applied with a few changes: - portOffset was not cached on the Connector (to align with recent changes to how port is handled) - handle special case port values where the offset should not be applied - use getPortWithOffset() everywhere - expose new attributes via JMX - take a slightly different approach with logging in case anyone (not that they should) is parsing log messages based on the current format - added handling for the redirectPort attribute - fix any IDE / CheckStyle warnings Despite what might look like a long list of changes, the patch was applied largely as-is. I particularly like the approach to handling the Connectors. Many thanks. This feature will be available in 9.0.13 onwards. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #130: Add two spaces (nbsp) in front of the Undeploy button
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/130 I can see what you are trying to do but I'm not sure the change creates enough of a visual separation to make much of a difference. I don't think it will do much to reduce the risk of an accidental click. On the other hand, it would have taken me less time to just apply the change than I have spent writing this comment. I'm going to apply the change as it won't make the issue worse, it might help and it is a trivial change. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #131: qqq
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/131 Spam. Reported to GitHub's abuse team. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #131: qqq
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/131 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #132: Fix cygpath error by adding cond check with JAVA_E...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/132 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #132: Fix cygpath error by adding cond check with JAVA_ENDORSED...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/132 Fixed in: - trunk for 9.0.14 onwards - 8.5.x for 8.5.36 onwards - 7.0.x for 7.0.93 onwards Thanks for the report and for the patch. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #134: Display generate date when showing every message in Html ...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/134 How easy it is to get hold of the last modified date does vary by browser but - given that Tomcat already provides this in the headers - I don't see much benefit here. And micheal-o's comment on checksumming is a valid one. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #134: Display generate date when showing every message in Html ...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/134 The browser history will give you what you need (to the minute anyway) in both Chrome Firefox and IE. This is a generic solution that works for any web page for any service - not just one page of the Tomcat Manager app. I'm still looking into why the browser doesn't show a last modified date. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #134: Display generate date when showing every message in Html ...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/134 I tried adding a Last-Modified HTTP header but that didn't help. The history approach looks to be the best solution at this point. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #74: added javadoc comment
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/74 There is definitely some value here although we might not choose to skip some parts of the patch. Let me pull in the obvious stuff and then we can see what is left. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #74: added javadoc comment
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/74 I think I've pulled the obvious wins into Tomcat. Take a look and if you think there is merit in further changes please feel free to open another pull request. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #74: added javadoc comment
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/74 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #63: added portOffset attribute to server.xml per BZ-61171
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/63 I've added some comments to the bugzilla issue on how this might be addressed. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #73: Bug 57767 - Websocket client proprietary configuration
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/73 Chris's original concern with the BZ 57767 patch (lack of Javadoc) still needs to be addressed. --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/76 I did wonder if this would be better as configuration on WsFilter but on reflection a separate Filter looks to be simpler for users to configure. The patch needs documentation (webapps/docs/config/filter.xml) No need to patches for the back-ports. We'll use svn merge and add NO-OP methods as necessary. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #77: Removed findbugs bad practice warnings by making classes f...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/77 Making those classes final will cause problems if users have extended any of them. While that seems unlikely, experience suggests it has probably been done somewhere. Is there any reason super.clone() can't be used instead for these? --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #78: added findbugs false positive
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/78 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #78: added findbugs false positive
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/78 Thanks. Patch applied to trunk and back-ported to 8.5.x, 8.0.x and 7.0.x. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #73: Bug 57767 - Websocket client proprietary configuration
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/73 No objections to back-porting here. --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/79 Those aren't placeholders. It is intentional content. I believe they are meant to be a lighthearted way of indicating that 'Context' is the only term that needs introducing. --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/79 We could pull the entries and add "That's it!" after the list. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #83: Bug 61668 - Possible NullPointerException in AbstractHttp1...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/83 The duplicated code is a sign that this could be handled elsewhere with less code (i.e. in StringUtils) --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #82: Add maven-install target
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/82 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #83: Bug 61668 - Possible NullPointerException in AbstractHttp1...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/83 Thanks for the proposed patch. I opted for a more general solution in StringUtils since fixing that problem there should - in theory - prevent it appearing anywhere else StringUtils is used. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #83: Bug 61668 - Possible NullPointerException in Abstra...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/83 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #84: Add tomcat in the cloud abstract implementation
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/84 This needs a wider discussion on the dev list first. However I do have some initial high level comments: - The choice of package is unexpected - Headers need to be standard ASF headers - Author details need to be removed - At least one file appears to have been written by someone other than the person submitting the pull request. That is a huge red flag as it is not clear that the person submitted the pull request has the necessary rights to do so. --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/76 Looking at this is on my TODO list to look at before the next release --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #73: Bug 57767 - Websocket client proprietary configurat...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/73 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #85: Fix NPE in AuthConfigFactoryImpl.detachListener()
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/85 Note there is a minor typo in the test method name. I'll fix that when I apply the patch. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #87: Fix NPE on removeRegistration in case getConfigProvider is...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/87 Applied in r1815802 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #87: Fix NPE on removeRegistration in case getConfigProv...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/87 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #95: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61223
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/95 This is a partial fix. It only addresses the attributes of the mbean element. I'm wondering if, rather than duplicate the comments in the DTD file, if it would be simpler to copy the DTD to the docs and link to the DTD. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #95: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61223
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/95 Thanks for the patch. There wasn't anything wrong with it but the more I thought about it, the more I thought that referencing the DTD directly was a better solution. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #95: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/95 --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/96 The current OPTIONS behaviour is intentional. OPTIONS lists the valid methods, not the permitted methods, for a resource. The GET/POST issue is before my time with the project. There might be some explanation in the mail archive. I agree with Chris it is 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 #96: Remove PUT and DELETE methods from an OPTIONS request if r...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/96 The inconsistency stems from the different status codes used. If TRACE is disabled at the connector, level then a 405 is returned for TRACE requests. If readOnly is set in the DefaultServlet, a 403 is returned for PUT and DELETE. Methods that return a 405 should not be included in an OPTIONS response. Doing some mailing list archaeology, the readOnly parameter started out in the WebDAV servlet ( http://markmail.org/message/vls7an5nhf7eigwc ) and was refactored to the Default Servlet shortly afterwards ( http://markmail.org/message/vls7an5nhf7eigwc ). All this was back in 2000. Looking at RFC 2616 (since this code pre-dates RFC7231) there isn't much to choose between 403 and 405. Both look equally applicable. One obvious difference for this use case is that a 405 MUST include an Allow header so on that basis using 403 looks to be the simpler implementation. RFC 7231 adds some detail on the differences between 403 and 405. In short 405 means 'method never allowed' whereas 403 means 'request not allowed but might be allowed with different credentials'. In this case it is arguable that 405 is the better response code since if readOnly is true, PUT and DELETE are never going to be allowed (with a similar argument applied to many of the WebDAV methods). We have made changes in 9.0.x to better align with RFC 7231 so I'm not against considering changing the status code used. That would also mean changing the OPTIONS response. As I have been researching this I noticed that the is scope to clean up the existing code. HttpServlet.doOptions() could use a StringBuilder and ALLOW_OPTIONS seems unnecessary. DefaultServlet.doOptions() appears to be unnecessary as well. This clean-up assumes the current behaviour is not changed. A switch to using 405 is likely to impact some of that clean-up. If there is interest in switching the DefaultServlet to use 405 rather than 403 when readOnly is true, I'd suggest that a discussion on the dev@ list is required to ensure there is consensus on this approach. --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/96 I wanted to clear something up. It is not a case of me being willing to change something or not. I don't get to decide these things on my own. It is a community decision. Normally, we discuss things until we reach a solution we can all accept. In the unlikely event we can't reach an agreed solution then the PMC would have to vote to resolve the impasse. It is extremely rare that things reach that point. --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/96 Getting back to whether readOnly should affect POST, my own view is that it should not. readOnly refers to whether the default Servlet can change static content. For static content request parameters (via GET or POST) are irrelevant since they are ignored. Hence why POST defers to GET. Neither change existign content hence neither should be affected by readOnly. If the default Servlet (or WebDAV since it extends the default Servlet) used the request parameters then it might (depending on how they were used) be different. --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/96 Lack of response == lazy consensus. I plan to look at applying these patches (or possibly a variation of them) before tagging 9.0.x. --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/96 No argument with Konstantin's points here. I started to look at implementing this and I realised that the WebDAV is inconsistent with how the allow header is generated. A method may not be included in the allow header but will not generate a 405 if that method is used for the same resource. That seems wrong to me. Before I start changing this, I'm going to put together a simple test case to check it and dig out the WebDAV test suite we have used in the past to make sure my changes don't break anything. This is probably going to delay the tag by a day or so. --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/96 I've fixed this in trunk. Given the behaviour changes (you can bet someone depends on a 403 rather than a 405 response) I'm not going to back-port it. The way I ended up tackling this (WebDAV first then Default) meant I didn't work from the patch but I certainly depended heavily on this discussion. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #135: fix bug 63050: NumberFormatException depending on ...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/135 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #135: fix bug 63050: NumberFormatException depending on languag...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/135 This has been fixed via [POEditor](https://poeditor.com/projects/view?id=221603) and will be included in 9.0.15 onwards. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #137: Add HTML lang="en"
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/137 I'm all in favour of improving accessibility. The changes to package.html and similar look good. The changes to anything under test/ need to be reverted. Those are test artefacts. The changes to to anything under webapps/ needs to be considered in the context of the work to improve localisation where there is at least one open enhancement to add the necessary framework to provide that content in languages with the correct language being selected based on the user's locale. It may be that the changes proposed here are OK. It just needs some thought. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #137: Add HTML lang="en"
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/137 > Anyway, those package.html files are just fragments consumed by javadoc tool. I suspect that changing the attributes of the HTML element does not have any effect on generated Javadoc documentation, but it needs testing. I took a quick look. You are correct. The package.html files become package-summary.html files and they all use HTML 4.0.1 and have lang="en" On that basis package.html changes are unnecessary and should also be removed from this PR. I also looked at the other HTML files and they don't seem to serve any purpose. I'll look into them some more but I'll probably end up removing them. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request: Recursively create document base directory
Github user markt-asf commented on the pull request: https://github.com/apache/tomcat/pull/20#issuecomment-106360093 Starting Tomcat with an appBase that is not an existing directory is an error and is reported as such in the logs. --- 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: Add JASPIC API (JSR 196) to build process
Github user markt-asf commented on the pull request: https://github.com/apache/tomcat/pull/21#issuecomment-109959429 OK. I've copied the Geronimo classes across and fixed the checkstyle and compilation errors. I've now seen some additional issues that need to be addressed. 1. Remove the @version lines from the Javadoc (and the whole comment if that is the only Javadoc present) 2. Line length (80 for comments, 100 for code) 3. Use imports (particularly noticed in AuthConfigFactory) And something I forgot in my comment on the commit, the integration into build.xml should be a separate commit to. --- 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: Add JASPIC API (JSR 196) to build process
Github user markt-asf commented on the pull request: https://github.com/apache/tomcat/pull/21#issuecomment-110393102 1) I haven't reviewed the build.xml changes since I don't want to start building this JAR until the API is correct. 2) Patch was incomplete. Applied along with additional patch for missing elements. 3) I'm not convinced (yet) of the need for this refactoring. And it appears to have broken the caching. 4) Patch was incomplete. Applied along with additional patch for missing elements. 5) I'm not convinced. At least one commit contained tabs which checkstyle would have rejected. 6) Look harder. For example. AuthConfigFactory has a number of additional constants. I typically use javap against both Tomcat and the reference implementation and diff the two results. I'll even go as far as re-ordering the methods in Tomcat to remove changes from the diff. When I ran this against the Geronimo classes there were a number of differences between Geronimo and the reference implementation. --- 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: Add JASPIC API (JSR 196) to build process
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/21 --- 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: [Bug 58242] Scanning jars in classpath to get...
Github user markt-asf commented on the pull request:
https://github.com/apache/tomcat/pull/22#issuecomment-130565904
The patch appears to be quite large for a marginal benfit. Separating the
refactoring and the changes into separate commits would enable the true scale
of the changes to be judged.
I remain on the fence about this enhancement, pending a patch that clearly
shows the scale of the changes.
I am -1 on the patch as current written since:
- it is not thread safe (look at the Java class cache)
- it does not follow the coding style of the existing code:
- no tabs (use 4 * spaces for indent)
- no new line before {
- no space after (
- no space before )
- it uses system properties for configuration when they are not necessary.
Configuration for this feature should be at the Context level
- no documentation provided
- no license header in new files
- max coding line length is 100
- max comment line length is 80
- it needs to be dsiabled by default
I also have some more general concerns about this feature:
- Is parallel threads scanning the same class an issue and if so, how do we
prevent that?
- The deployment process is already (or can be) multi-threaded. How much
benefit does this feature provide when multiple apps are being deployed in
parallel?
- What will be the impact on performance for already deployed applications
if this feature is enabled?
- Is a 20s default tieout to scan a JAR appropriate?
---
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: [Bug 58242] Scanning jars in classpath to get...
Github user markt-asf commented on the pull request: https://github.com/apache/tomcat/pull/22#issuecomment-131576391 My comment regarding separating the refactoring and the functional changes stands. It is not the refactoring I am against, it is the mixing of refactoring and functional change in a single commit. /res/checkstyle Not every rule I mentioned is enforced mainly because not all of the current code follows it (due to the age of the code). We could go through and make all the changes but that creates a different problem (back-ports get more difficult). The general rule is a patch should follow the style of the code it is changing. Regarding thread safety I can now see that my concern regarding the Java class cache was mis-placed. However, scanning an individual class triggers the scanning of all the classes in the hierarchy. If those classes are in different JARs then you will get the same class scanned by multiple threads. How much of an issue that is is TBD. While this feature is likely to improve the startup of a Tomcat instance with a single, large application deployed on a multi-core machine I am far from convinced it will help when there are multiple applications deployed particularly when there are more applications deployed than cores available. Your lack of concern for the impact this feature may have on running applications is worring. As currently written it is likely that a deployment using this feature would have a noticeable performance impact on running applications if the system was moderately loaded at the time. My position remains -1 on the proposed patch and neutral on the feature. --- 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: Added support for addWebapp(String, InputStre...
Github user markt-asf commented on the pull request: https://github.com/apache/tomcat/pull/19#issuecomment-87757450 -1. Servlet 3.0 and later webapps have resource JARs (assuming the JAR is packaged correctly). For single JAR apps, the Tomcat 8 resources implementation can be used to serve JSPs from any location within a JAR. Supporting this in Tomcat 7 would require a new DirContext impl. Given how fragile that code is I'd be extremely reluctant to make any changes to it that were not strictly 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 pull request: Added support for addWebapp(String, InputStre...
Github user markt-asf commented on the pull request: https://github.com/apache/tomcat/pull/19#issuecomment-87983701 No. That is not what I am saying at all. I am saying the functionality you state you want to add (serving JSPs from within a JAR) can already be achieved by existing functionality for Tomcat 8 and trunk. Tomcat 7 would require a new DirContext implementation (which is likely to be very risky). --- 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: now json is fashion
Github user markt-asf commented on the pull request: https://github.com/apache/tomcat/pull/15#issuecomment-69624704 This pull request is declined. Even ignoring the poor quality of this pull request, supporting JSON formatted data is an application concern, not a container concern. --- 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 #58: add nioendpoint note
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/58 Given the name of the method call, the proposed comment adds no value. The patch also uses tabs rather than spaces. --- 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
