You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2013/01/07 21:18:10 UTC
svn commit: r1429991 - in /tomcat/tc7.0.x/trunk:
java/org/apache/naming/resources/DirContextURLConnection.java
webapps/docs/changelog.xml
Author: markt
Date: Mon Jan 7 20:18:09 2013
New Revision: 1429991
URL: http://svn.apache.org/viewvc?rev=1429991&view=rev
Log:
Follow-up for r1429858.
Further fixes for https://issues.apache.org/bugzilla/show_bug.cgi?id=54371
Handle the additional cases that r1429858 did not ('~', '+', ';' and '&' characters in the context path) and simplify at the same time.
Modified:
tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/DirContextURLConnection.java
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
Modified: tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/DirContextURLConnection.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/DirContextURLConnection.java?rev=1429991&r1=1429990&r2=1429991&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/DirContextURLConnection.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/DirContextURLConnection.java Mon Jan 7 20:18:09 2013
@@ -435,28 +435,8 @@ public class DirContextURLConnection ext
if (collection != null) {
try {
- String file = getURL().getFile();
- // This will be of the form /<hostname>/<contextpath>/file name
- // if <contextpath> is not empty otherwise this will be of the
- // form /<hostname>/file name
- // Strip off the hostname and the contextpath (note that context
- // path may contain '/'
- int start;
- if (context instanceof ProxyDirContext) {
- String cp = ((ProxyDirContext)context).getContextPath();
- cp = URL_ENCODER.encodeURL(cp);
- String h = ((ProxyDirContext)context).getHostName();
- if ("".equals(cp)) {
- start = h.length() + 2;
- } else {
- start = h.length() + cp.length() + 2;
- }
- } else {
- start = file.indexOf('/', file.indexOf('/', 1) + 1);
- }
-
NamingEnumeration<NameClassPair> enumeration =
- context.list(file.substring(start));
+ collection.list("/");
while (enumeration.hasMoreElements()) {
NameClassPair ncp = enumeration.nextElement();
result.addElement(URL_ENCODER.encodeURL(ncp.getName()));
Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1429991&r1=1429990&r2=1429991&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Mon Jan 7 20:18:09 2013
@@ -106,7 +106,7 @@
<fix>
<bug>54371</bug>: Prevent exceptions when processing web fragments for
unexpanded WAR files when the context path contains characters that
- need to be encoded in URLs such as spaces. Patch provided by Polina
+ need to be encoded in URLs such as spaces. Based on a patch by Polina
Genova. (markt)
</fix>
</changelog>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1429991 - in /tomcat/tc7.0.x/trunk:
java/org/apache/naming/resources/DirContextURLConnection.java webapps/docs/changelog.xml
Posted by Konstantin Kolinko <kn...@gmail.com>.
2013/1/8 Mark Thomas <ma...@apache.org>:
> On 08/01/2013 09:30, Konstantin Kolinko wrote:
>> 2013/1/8 <ma...@apache.org>:
>>> Author: markt
>>> Date: Mon Jan 7 20:18:09 2013
>>> New Revision: 1429991
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1429991&view=rev
>>> Log:
>>> Follow-up for r1429858.
>>> Further fixes for https://issues.apache.org/bugzilla/show_bug.cgi?id=54371
>>> Handle the additional cases that r1429858 did not ('~', '+', ';' and '&' characters in the context path) and simplify at the same time.
>
> <snip/>
>
>> 1. I have not tested it yet, but I doubt that this works for Tomcat 7.
>>
>> See BaseDirContext.list(String) how it processes the aliases and
>> altDirContexts there. I think that calling collection.list() skips
>> that processing.
>
> It is no worse than the current code.
>
> Consider the following:
>
> You use an empty directory as the basis of a web application.
> You add an alias for WEB-INF/lib which points to a directory full of JARs.
>
> list("/") is going to return an empty set when you might expect it to
> return a single entry for WEB-INF.
>
> I'm not particularly keen on trying to fix this particular issue as the
> code is already rather fragile and there is a simple workaround: make
> sure that the root of your alias (WEB-INF/lib in this case) exists in
> the base directory.
>
> I will look at this for the new Tomcat 8 resources implementation to see
> how it handles it and if any improvements are required.
>
OK.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1429991 - in /tomcat/tc7.0.x/trunk: java/org/apache/naming/resources/DirContextURLConnection.java
webapps/docs/changelog.xml
Posted by Mark Thomas <ma...@apache.org>.
On 08/01/2013 09:30, Konstantin Kolinko wrote:
> 2013/1/8 <ma...@apache.org>:
>> Author: markt
>> Date: Mon Jan 7 20:18:09 2013
>> New Revision: 1429991
>>
>> URL: http://svn.apache.org/viewvc?rev=1429991&view=rev
>> Log:
>> Follow-up for r1429858.
>> Further fixes for https://issues.apache.org/bugzilla/show_bug.cgi?id=54371
>> Handle the additional cases that r1429858 did not ('~', '+', ';' and '&' characters in the context path) and simplify at the same time.
<snip/>
> 1. I have not tested it yet, but I doubt that this works for Tomcat 7.
>
> See BaseDirContext.list(String) how it processes the aliases and
> altDirContexts there. I think that calling collection.list() skips
> that processing.
It is no worse than the current code.
Consider the following:
You use an empty directory as the basis of a web application.
You add an alias for WEB-INF/lib which points to a directory full of JARs.
list("/") is going to return an empty set when you might expect it to
return a single entry for WEB-INF.
I'm not particularly keen on trying to fix this particular issue as the
code is already rather fragile and there is a simple workaround: make
sure that the root of your alias (WEB-INF/lib in this case) exists in
the base directory.
I will look at this for the new Tomcat 8 resources implementation to see
how it handles it and if any improvements are required.
Mark
> 2. I wonder whether "." or "" would be better instead of "/".
>
> WARDirContext.doListBindings(String) works faster if name.isEmpty(),
> as one lookup step is skipped.
>
> Best regards,
> Konstantin Kolinko
>
>> while (enumeration.hasMoreElements()) {
>> NameClassPair ncp = enumeration.nextElement();
>> result.addElement(URL_ENCODER.encodeURL(ncp.getName()));
>>
>> Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>> URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1429991&r1=1429990&r2=1429991&view=diff
>> ==============================================================================
>> --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
>> +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Mon Jan 7 20:18:09 2013
>> @@ -106,7 +106,7 @@
>> <fix>
>> <bug>54371</bug>: Prevent exceptions when processing web fragments for
>> unexpanded WAR files when the context path contains characters that
>> - need to be encoded in URLs such as spaces. Patch provided by Polina
>> + need to be encoded in URLs such as spaces. Based on a patch by Polina
>> Genova. (markt)
>> </fix>
>> </changelog>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1429991 - in /tomcat/tc7.0.x/trunk:
java/org/apache/naming/resources/DirContextURLConnection.java webapps/docs/changelog.xml
Posted by Konstantin Kolinko <kn...@gmail.com>.
2013/1/8 <ma...@apache.org>:
> Author: markt
> Date: Mon Jan 7 20:18:09 2013
> New Revision: 1429991
>
> URL: http://svn.apache.org/viewvc?rev=1429991&view=rev
> Log:
> Follow-up for r1429858.
> Further fixes for https://issues.apache.org/bugzilla/show_bug.cgi?id=54371
> Handle the additional cases that r1429858 did not ('~', '+', ';' and '&' characters in the context path) and simplify at the same time.
>
> Modified:
> tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/DirContextURLConnection.java
> tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>
> Modified: tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/DirContextURLConnection.java
> URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/DirContextURLConnection.java?rev=1429991&r1=1429990&r2=1429991&view=diff
> ==============================================================================
> --- tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/DirContextURLConnection.java (original)
> +++ tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/DirContextURLConnection.java Mon Jan 7 20:18:09 2013
> @@ -435,28 +435,8 @@ public class DirContextURLConnection ext
>
> if (collection != null) {
> try {
> - String file = getURL().getFile();
> - // This will be of the form /<hostname>/<contextpath>/file name
> - // if <contextpath> is not empty otherwise this will be of the
> - // form /<hostname>/file name
> - // Strip off the hostname and the contextpath (note that context
> - // path may contain '/'
> - int start;
> - if (context instanceof ProxyDirContext) {
> - String cp = ((ProxyDirContext)context).getContextPath();
> - cp = URL_ENCODER.encodeURL(cp);
> - String h = ((ProxyDirContext)context).getHostName();
> - if ("".equals(cp)) {
> - start = h.length() + 2;
> - } else {
> - start = h.length() + cp.length() + 2;
> - }
> - } else {
> - start = file.indexOf('/', file.indexOf('/', 1) + 1);
> - }
> -
> NamingEnumeration<NameClassPair> enumeration =
> - context.list(file.substring(start));
> + collection.list("/");
1. I have not tested it yet, but I doubt that this works for Tomcat 7.
See BaseDirContext.list(String) how it processes the aliases and
altDirContexts there. I think that calling collection.list() skips
that processing.
2. I wonder whether "." or "" would be better instead of "/".
WARDirContext.doListBindings(String) works faster if name.isEmpty(),
as one lookup step is skipped.
Best regards,
Konstantin Kolinko
> while (enumeration.hasMoreElements()) {
> NameClassPair ncp = enumeration.nextElement();
> result.addElement(URL_ENCODER.encodeURL(ncp.getName()));
>
> Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
> URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1429991&r1=1429990&r2=1429991&view=diff
> ==============================================================================
> --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Mon Jan 7 20:18:09 2013
> @@ -106,7 +106,7 @@
> <fix>
> <bug>54371</bug>: Prevent exceptions when processing web fragments for
> unexpanded WAR files when the context path contains characters that
> - need to be encoded in URLs such as spaces. Patch provided by Polina
> + need to be encoded in URLs such as spaces. Based on a patch by Polina
> Genova. (markt)
> </fix>
> </changelog>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org