[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 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 #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 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 #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 #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 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 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 #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 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 #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 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 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 #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 #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 #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 #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 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 #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 #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 #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 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 #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 #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 #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 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_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 #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 #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 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 #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 #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 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 These have been resolved since this PR was opened. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #77: Removed findbugs bad practice warnings by making cl...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/77 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #49: Fix parser to fail if leading zeros in IPv4 part of IPv6 a...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/49 Thanks for the patch. Sorry it took a while to apply it. Due to our delays I had to adapt things a little. Fixed in: - trunk for 9.0.9 onwards - 8.5.x for 8.5.32 onwards - 8.0.x for 8.0.53 onwards - 7.0.x for 7.0.89 onwards --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #49: Fix parser to fail if leading zeros in IPv4 part of...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/49 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #111: Add ipv6 loopback address to the RemoteIpFilter and Remot...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/111 The docs (`config/valve.xml` and `config/filter.xml`) need to be updated with the new defaults as well. --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/108 I agree this should configurable on the Host. It should be possible to remove the Comparator entirely if the version code is used directly in the Mapper. I'd like this to be more robust. I'm thinking pad any dot-separated segment to a given length with a suitable character (possible space). --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #109: fix: file resource read dir inputStream
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/109 I came very close to fixing this but in the end I opted not to. I covered the argument for making this change in my previous comment. The reasons against it are: - it would be a potentially significant API change for 8.0.x, 8.5.x and 9.0.x - the behaviour isn't formally specified anywhere and has obvious problems with more unusual file names - My instinct is that null is a better option here This is probably another issue to take to the Servlet EG once it starts up at Eclipse for clarification. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #109: fix: file resource read dir inputStream
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/109 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #109: fix: file resource read dir inputStream
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/109 The specification is not as explicit as it could be in this case. The spec text refers to the Javadoc. The Javadoc says "Returns the resource located at the named path as an InputStream object." without any details of how a directory object should be represented as an InputStream. The Javadoc does include references to URL handlers and URLConnection objects so it is not unreasonable to assume that similar behaviour is expected. One concern that I do have with replicating the URLConnection behaviour is that it is not designed for file names that include \n. The are passed on without escaping and \n is also used as the separator between file names. However, that usage is rare and it might be a limitation we have to live with. There are other APIs that can be used to get directory listings without ambiguity. Rather than going via URLConnection, I'm leaning towards replicating the behaviour. I do want to check if the \n is always used or if it varies with platform. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #109: fix: file resource read dir inputStream
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/109 I'm not convinced the proposed approach is the right one in this case. --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/108 I think there is a lot of merit in Chris's idea. The concept of some sort of internal pre-computed version string that addresses ordering concerns by zero padding numerical components is one that would be easy to implement and have minimal impact on the existing system. --- - 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 markt-asf commented on the issue: https://github.com/apache/tomcat/pull/108 I have applied the optimisation commit (thanks) but not the ordering changes. The patch changes the order for undeploy without changing the order for version mapping. This will lead to unexpected behaviour. The current simple `String` based ordering was selected as a trade off between performance and usability. The type of ordering proposed in this patch was considered too expensive to incur on every request when multiple versions are deployed. To consider such a change, we'd need to see evidence of the performance impact (including GC) on the mapping process of switching to this ordering mechanism. Be aware that mapping code is highly optimised for `String`. Reviewing the proposed patch I see several things that would need to be fixed in additional to the more architectural issue described above: - imports from sun.* are not allowed - the code is Java 8 specific - any fix needs to be available to back-ported to 7.0.x will has to run on Java 6 - the patch does not compile - `Pattern` instances should be pre-compiled Finally, the patch only considers purely numeric versions. Many projects include a test string (e.g. RELEASE) in the version number. It would be nice to handle these as well. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #105: Iamtjw patch 1
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/105 This needs some discussion on the dev@ list. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #102: Fix Possible infinite loop in AprEndpoint.Poller#r...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/102 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #101: Fix compressibleMimeType handling for compression=...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/101 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #101: Fix compressibleMimeType handling for compression="on"
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/101 Thanks for the report and the patch. Fixed for trunk and 8.5.x. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #97: Bz48672
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/97 --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #97: Bz48672
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/97 All updated. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #97: Bz48672
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/97 Thanks looks great. I'll add it shortly. I'll apply to all versions and then tweak the various version specific parts so they reflect the current version. --- - 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 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 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 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 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 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 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 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 #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 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 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 #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 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 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 #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 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 #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 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 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 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 #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 #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 #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 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 #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 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 #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 #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 #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 #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 Generally, I'm in favour of refactoring that reduces the overall volume of code. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #75: added javadoc comments and a method that takes default val...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/75 -1 The public API of specification defined classes may not be changed. This method would be worth adding to o.a.catalina.filters.FilterBase if any of Tomcat's internal filter implementations could make use of it (i.e. if it resulted in removing more code than it added) --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #72: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=59901
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/72 My point is that with the o.a.c.webresources.Cache in place, this solution offers little additional benefit. I'm not suggesting creating dependencies from Jasper to Catalina. On closer inspection, caching the content in the ParserController does limit the lifetime of the cache appropriately. There are more than 2 reads per file. The process to determine encoding is another read. My thinking is really just a different solution. It just caches the content in a different place - a buffered input stream. From memory there was also the opportunity to reduce some unnecessary processing (the result would be unchanged from the #parseDirectives phase) in the #parse phase. --- 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 #72: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=59901
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/72 Tomcat provides (by default) a static content cache in the resources layer so I do wonder if there is much benefit in caching the content in the JSP engine as well. The proposed cache appears to be unbounded which is likely to be a problem for larger installations. Also, there is no need to cache the JSP content once it has been processed. My original thinking was more along the lines of some refactoring to allow re-use of a single buffered input stream with some limit on the buffer size that could mean the stream did need to be re-opened for large JSPs. It may be that the static content cache provided in the resources layer provides sufficient benefit that tidying up the multiple input streams provides minimal additional benefit in most cases. Where I would expect it to help is the case when (for whatever reason) caching in the resource layer is disabled. --- 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 #70: Add new accesslog valve pattern %X for recording connectio...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/70 Patch applied. Many 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 #70: Add new accesslog valve pattern %X for recording co...
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/70 --- 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 #70: Add new accesslog valve pattern %X for recording co...
Github user markt-asf commented on a diff in the pull request:
https://github.com/apache/tomcat/pull/70#discussion_r129111529
--- Diff: java/org/apache/catalina/valves/AbstractAccessLogValve.java ---
@@ -1506,6 +1508,38 @@ public void addElement(CharArrayWriter buf, Date
date, Request request,
}
}
+/**
+ * Write connection status when response is completed - %X
+ */
+protected static class ConnectionStatusElement implements
AccessLogElement {
+@Override
+public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
+if (response != null && request != null) {
+// Check for connection aborted cond
+boolean isConnAborted = false;
+if (response.isError()) {
+Throwable ex =
(Throwable)request.getAttribute(RequestDispatcher.ERROR_EXCEPTION);
+if (ex instanceof ClientAbortException) {
+isConnAborted = true;
+buf.append('X');
+}
+}
+
+// Check whether connection is keep-alive or not
+if (!isConnAborted) {
+if
(org.apache.coyote.http11.Constants.KEEPALIVE.equals(
+
request.getHeader(org.apache.coyote.http11.Constants.CONNECTION))) {
--- End diff --
The log message is intended to show if the connection remains in keep-alive
after the current request finishes. You want to look at the Connection header
(for close) on the response here to mimic the behaviour described for httpd.
---
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 #5: Add maxStartTime to RequestInfo & RequestGroupInfo.
Github user markt-asf closed the pull request at: https://github.com/apache/tomcat/pull/5 --- 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 #5: Add maxStartTime to RequestInfo & RequestGroupInfo.
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/5 As requested. Note: normally projects can't close PRs like this but I can because of my infra role. --- 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 #68: Fix #61224 - Make the GlobalRequestProcessor mbeans read-o...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/68 Tomcat fails to compile after applying this patch. Looking in to this, I don't see any reason for BaseModelMBean to throw exceptions in its constructor. As far as I can tell, the constructor (and the matching constructors on all the sub-classes) can be removed. I'm going to look into this before coming back to this PR. No need to update the PR. The changes required are minor enough that I'll handle it on merge. --- 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 #65: Fix bug #61214 : NoSuchMethodException in JMX Proxy (Conte...
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/65 My favourite kind of patch. It fixes a problem by deleting stuff. :) --- 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 #59: add some useful note
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/59 Most added comments add no value. The s/poller/pollers/ change is the only one worth considering and that is a trivial change. The patch uses a mix of tabs and spaces for indents. --- 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
