[GitHub] tomcat pull request #132: Fix cygpath error by adding cond check with JAVA_E...

2018-11-10 Thread zemian
GitHub user zemian opened a pull request:

https://github.com/apache/tomcat/pull/132

Fix cygpath error by adding cond check with JAVA_ENDORSED_DIRS

The new `bin/tool-wrapper.sh` script gives the error `cygpath: can't 
convert empty path` when used in Cygwin shell. This is due to 
`JAVA_ENDORSED_DIRS` was converted even when it's empty.

This PR fixes the error by checking if `JAVA_ENDORSED_DIRS` is not empty 
first.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zemian/tomcat zemian-cygpath-error

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tomcat/pull/132.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #132


commit 7d71bb72e64ac5a9e167f2ad71eb44a7183e2d1a
Author: Zemian Deng 
Date:   2018-11-11T00:15:50Z

Fix cygpath error by adding cond check with JAVA_ENDORSED_DIRS




---

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[GitHub] tomcat pull request #106: Fix cygpath is checking for empty path

2018-04-04 Thread zemian
GitHub user zemian opened a pull request:

https://github.com/apache/tomcat/pull/106

Fix cygpath is checking for empty path

Hi, When running Tomcat under Cygwin shell, I see `cygpath: can't convert 
empty path` error message printed, but it will continue to work. Looking 
closely, it seems that `catalina.sh` script didn't check for empty string for 
the optional JAVA_ENDORSED_DIRS var before calling cygpath. I have created a PR 
that fix this minor bug. Please review.

```
$ bin/catalina.sh version
cygpath: can't convert empty path
Using CATALINA_BASE:   C:\Users\zemian\apps\apache-tomcat-8.5.29
Using CATALINA_HOME:   C:\Users\zemian\apps\apache-tomcat-8.5.29
Using CATALINA_TMPDIR: C:\Users\zemian\apps\apache-tomcat-8.5.29\temp
Using JRE_HOME:C:\Users\zemian\apps\jdk-8u161
Using CLASSPATH:   
C:\Users\zemian\apps\apache-tomcat-8.5.29\bin\bootstrap.jar;C:\Users\zemian\apps\apache-tomcat-8.5.29\bin\tomcat-juli.jar
Server version: Apache Tomcat/8.5.29
Server built:   Mar 5 2018 13:11:12 UTC
Server number:  8.5.29.0
OS Name:Windows 7
OS Version: 6.1
Architecture:   amd64
JVM Version:1.8.0_161-b12
JVM Vendor: Oracle Corporation
```

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zemian/tomcat zemian_fix-cygpath

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tomcat/pull/106.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #106


commit 2534d5c3e6359e700690168998d8bd25c273b9b1
Author: Zemian Deng <zemian.deng@...>
Date:   2018-04-04T14:43:02Z

Fix cygpath is checking for empty path




---

-
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

2017-08-09 Thread zemian
Github user zemian commented on the issue:

https://github.com/apache/tomcat/pull/72
  
Okay Mark. At this point, I am not sure I have a full grasp of the solution 
you wanted yet. I will study little more and revisit this issue later.



---
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

2017-08-07 Thread zemian
Github user zemian commented on the issue:

https://github.com/apache/tomcat/pull/72
  
Hi Mark,

> 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.

Are you referring to `org.apache.catalina.webresources.Cache`? To me this 
enhancement is at jasper level, so I thought it make more sense to keep it self 
contained. Also, is okay to bring catalina classes into jasper package? But any 
rate, if you do decide we go this route, please confirm and I will look into it 
more.

> 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.

I thought about this, but my current proposed cache is already bounded by 
`ParserController` instance, and it is already per request only. You still 
think we should limit cache further? If we do this, then we add much more 
complexity to caching though. Please confirm if we need this.

> 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.

I am not clear on this suggestion yet. Do tell me more if you strongly feel 
that's more the right direction. My current proposed solution is to cache the 
jsp file content as char array, then re-feed into different Stream/Reader that 
is used by jasper parser. The previous code will perform two reads per single 
jsp file processing: once for `ParserController#parseDirectives` and another 
for `JspDocumentParser#parse`. The proposed code will reduce these to one file 
read only. 



---
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 #72: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=...

2017-08-06 Thread zemian
GitHub user zemian opened a pull request:

https://github.com/apache/tomcat/pull/72

Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=59901

Caching jsp file read to improve performance

Hi, I added jsp file content cache inside ParserController for this 
enhancement. This reduced two read I found during the parsing for both XML jsp 
and standard jsp. 

Impl notes: The ParserController instance seems to be bounded to 
JspServletWrapper lifecycle though, so this will cache only per 
JspServletWrapper instance context as well. I have seen that some tag file 
processing will create nested instances of JspServletWrapper, and in this case, 
we still not able to use the cache. I wasn't sure was there a better place to 
add the cache that can reuse by multiple JspServletWrapper instances at this 
point. Please let me know what you think what you think so far.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zemian/tomcat zemian_59901-jsp-file-cache

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tomcat/pull/72.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #72


commit 9dff7cf58645eb0e2f2b08333b225373d9144361
Author: Zemian Deng <zemian.d...@bnymellon.com>
Date:   2017-08-06T21:09:07Z

Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=59901

Caching jsp file read to improve performance




---
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...

2017-07-20 Thread zemian
GitHub user zemian opened a pull request:

https://github.com/apache/tomcat/pull/70

Add new accesslog valve pattern %X for recording connection status

- https://bz.apache.org/bugzilla/show_bug.cgi?id=61164

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zemian/tomcat zemian_61164-accesslog

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tomcat/pull/70.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #70


commit 63e749b4d57c9d38ce2b6dc3e8fd54f200f60d61
Author: Zemian Deng <zemian.d...@bnymellon.com>
Date:   2017-07-21T03:10:53Z

Add new accesslog valve pattern %X for recording connection status

- https://bz.apache.org/bugzilla/show_bug.cgi?id=61164




---
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