Re: svn commit: r1823460 - in /tomcat/tc7.0.x/trunk: java/org/apache/catalina/loader/WebappClassLoaderBase.java webapps/docs/changelog.xml
Am 07.02.2018 um 17:51 schrieb Violeta Georgieva: 2018-02-07 18:39 GMT+02:00 Rainer Jung : Am 07.02.2018 um 16:37 schrieb Rainer Jung: Thanks for the review. I will look into it. Do you think we can keep it for this release, based on the experience from previous trunk and TC 8.0/tc 8.5 releases? Other options rare reverting, or applying a change in trunk etc. very soon, or delaying the 7.0 release. I can look into it today, but it might take until tomorrw before I am sure enough to change this. Found the time to do it right now. Already committed. I removed those checks and decided to also switch to using a sub string from the already available "path" instead of again replacing all occurrences of "." by "/". Are we ready for release? From my point of view "yes". Regards, Rainer - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r1823460 - in /tomcat/tc7.0.x/trunk: java/org/apache/catalina/loader/WebappClassLoaderBase.java webapps/docs/changelog.xml
On 07/02/18 16:51, Violeta Georgieva wrote: > 2018-02-07 18:39 GMT+02:00 Rainer Jung : >> >> Am 07.02.2018 um 16:37 schrieb Rainer Jung: >>> >>> Thanks for the review. I will look into it. Do you think we can keep it > for this release, based on the experience from previous trunk and TC 8.0/tc > 8.5 releases? Other options rare reverting, or applying a change in trunk > etc. very soon, or delaying the 7.0 release. >>> >>> I can look into it today, but it might take until tomorrw before I am > sure enough to change this. >> >> >> Found the time to do it right now. Already committed. >> >> I removed those checks and decided to also switch to using a sub string > from the already available "path" instead of again replacing all > occurrences of "." by "/". > > Are we ready for release? I'm done with the documentation changes. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r1823460 - in /tomcat/tc7.0.x/trunk: java/org/apache/catalina/loader/WebappClassLoaderBase.java webapps/docs/changelog.xml
2018-02-07 18:39 GMT+02:00 Rainer Jung : > > Am 07.02.2018 um 16:37 schrieb Rainer Jung: >> >> Thanks for the review. I will look into it. Do you think we can keep it for this release, based on the experience from previous trunk and TC 8.0/tc 8.5 releases? Other options rare reverting, or applying a change in trunk etc. very soon, or delaying the 7.0 release. >> >> I can look into it today, but it might take until tomorrw before I am sure enough to change this. > > > Found the time to do it right now. Already committed. > > I removed those checks and decided to also switch to using a sub string from the already available "path" instead of again replacing all occurrences of "." by "/". Are we ready for release? Thanks, Violeta > > Regards, > > Rainer > > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org >
Re: svn commit: r1823460 - in /tomcat/tc7.0.x/trunk: java/org/apache/catalina/loader/WebappClassLoaderBase.java webapps/docs/changelog.xml
Am 07.02.2018 um 16:37 schrieb Rainer Jung: Thanks for the review. I will look into it. Do you think we can keep it for this release, based on the experience from previous trunk and TC 8.0/tc 8.5 releases? Other options rare reverting, or applying a change in trunk etc. very soon, or delaying the 7.0 release. I can look into it today, but it might take until tomorrw before I am sure enough to change this. Found the time to do it right now. Already committed. I removed those checks and decided to also switch to using a sub string from the already available "path" instead of again replacing all occurrences of "." by "/". Regards, Rainer - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r1823460 - in /tomcat/tc7.0.x/trunk: java/org/apache/catalina/loader/WebappClassLoaderBase.java webapps/docs/changelog.xml
Am 07.02.2018 um 15:39 schrieb Konstantin Kolinko: 2018-02-07 14:54 GMT+03:00 : Author: rjung Date: Wed Feb 7 11:54:45 2018 New Revision: 1823460 URL: http://svn.apache.org/viewvc?rev=1823460&view=rev Log: BZ58143: Fix calling classloading transformers broken in 7.0.70 by the fix for BZ59619. This was observed when using Spring weaving. Partial backport of r1730946, follow on to r1745608. Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java?rev=1823460&r1=1823459&r2=1823460&view=diff == --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java Wed Feb 7 11:54:45 2018 @@ -3075,6 +3075,28 @@ public abstract class WebappClassLoaderB if (entry.binaryContent == null) throw new ClassNotFoundException(name); +if (this.transformers.size() > 0) { +// If the resource is a class just being loaded, decorate it +// with any attached transformers +String className = name.endsWith(CLASS_FILE_SUFFIX) ? +name.substring(0, name.length() - CLASS_FILE_SUFFIX.length()) : name; +String internalName = className.replace(".", "/"); The above lines do not make much sense for me here. (It is not a fault of this specific commit. The same code is in trunk). These lines of code were in the findResourceInternal() method below that deals with resources. They are now moved here into the findClassInternal() method that deals with classes. The class names do not have a CLASS_FILE_SUFFIX and the substring() call is not needed. A "String path = binaryNameToPath(name, true);" call several lines above always adds the suffix. The original name does not have it. +for (ClassFileTransformer transformer : this.transformers) { +try { +byte[] transformed = transformer.transform( +this, internalName, null, null, entry.binaryContent +); +if (transformed != null) { +entry.binaryContent = transformed; +} +} catch (IllegalClassFormatException e) { + log.error(sm.getString("webappClassLoader.transformError", name), e); +return null; +} +} +} + // Looking up the package String packageName = null; int pos = name.lastIndexOf('.'); @@ -3477,29 +3499,6 @@ public abstract class WebappClassLoaderB } } } - -if (isClassResource && entry.binaryContent != null && -this.transformers.size() > 0) { -// If the resource is a class just being loaded, decorate it -// with any attached transformers -String className = name.endsWith(CLASS_FILE_SUFFIX) ? -name.substring(0, name.length() - CLASS_FILE_SUFFIX.length()) : name; -String internalName = className.replace(".", "/"); - -for (ClassFileTransformer transformer : this.transformers) { -try { -byte[] transformed = transformer.transform( -this, internalName, null, null, entry.binaryContent -); -if (transformed != null) { -entry.binaryContent = transformed; -} -} catch (IllegalClassFormatException e) { -log.error(sm.getString("webappClassLoader.transformError", name), e); -return null; -} -} -} // Add the entry in the local resource repository synchronized (resourceEntries) { 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=1823460&r1=1823459&r2=1823460&view=diff == --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Wed Feb 7 11:54:45 2018 @@ -72,6 +72,11 @@ (remm) +58143: Fix calling classloading transformers broken in 7.0.70 +by the fix for 59619. This was observed when using Spring +weaving. (rjung) + + 62000: When a JNDI referen
Re: svn commit: r1823460 - in /tomcat/tc7.0.x/trunk: java/org/apache/catalina/loader/WebappClassLoaderBase.java webapps/docs/changelog.xml
2018-02-07 14:54 GMT+03:00 : > Author: rjung > Date: Wed Feb 7 11:54:45 2018 > New Revision: 1823460 > > URL: http://svn.apache.org/viewvc?rev=1823460&view=rev > Log: > BZ58143: Fix calling classloading transformers broken in 7.0.70 > by the fix for BZ59619. This was observed when using Spring > weaving. > > Partial backport of r1730946, follow on to r1745608. > > Modified: > > tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java > tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml > > Modified: > tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java > URL: > http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java?rev=1823460&r1=1823459&r2=1823460&view=diff > == > --- > tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java > (original) > +++ > tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java > Wed Feb 7 11:54:45 2018 > @@ -3075,6 +3075,28 @@ public abstract class WebappClassLoaderB > if (entry.binaryContent == null) > throw new ClassNotFoundException(name); > > +if (this.transformers.size() > 0) { > +// If the resource is a class just being loaded, decorate it > +// with any attached transformers > +String className = name.endsWith(CLASS_FILE_SUFFIX) ? > +name.substring(0, name.length() - > CLASS_FILE_SUFFIX.length()) : name; > +String internalName = className.replace(".", "/"); The above lines do not make much sense for me here. (It is not a fault of this specific commit. The same code is in trunk). These lines of code were in the findResourceInternal() method below that deals with resources. They are now moved here into the findClassInternal() method that deals with classes. The class names do not have a CLASS_FILE_SUFFIX and the substring() call is not needed. A "String path = binaryNameToPath(name, true);" call several lines above always adds the suffix. The original name does not have it. > +for (ClassFileTransformer transformer : this.transformers) { > +try { > +byte[] transformed = transformer.transform( > +this, internalName, null, null, > entry.binaryContent > +); > +if (transformed != null) { > +entry.binaryContent = transformed; > +} > +} catch (IllegalClassFormatException e) { > + > log.error(sm.getString("webappClassLoader.transformError", name), e); > +return null; > +} > +} > +} > + > // Looking up the package > String packageName = null; > int pos = name.lastIndexOf('.'); > @@ -3477,29 +3499,6 @@ public abstract class WebappClassLoaderB > } > } > } > - > -if (isClassResource && entry.binaryContent != null && > -this.transformers.size() > 0) { > -// If the resource is a class just being loaded, decorate it > -// with any attached transformers > -String className = name.endsWith(CLASS_FILE_SUFFIX) ? > -name.substring(0, name.length() - > CLASS_FILE_SUFFIX.length()) : name; > -String internalName = className.replace(".", "/"); > - > -for (ClassFileTransformer transformer : this.transformers) { > -try { > -byte[] transformed = transformer.transform( > -this, internalName, null, null, > entry.binaryContent > -); > -if (transformed != null) { > -entry.binaryContent = transformed; > -} > -} catch (IllegalClassFormatException e) { > - > log.error(sm.getString("webappClassLoader.transformError", name), e); > -return null; > -} > -} > -} > > // Add the entry in the local resource repository > synchronized (resourceEntries) { > > 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=1823460&r1=1823459&r2=1823460&view=diff > == > --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original) > +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Wed Feb 7 11:54:45 2018 > @@ -72,6 +72,11 @@ > (remm) > > > +58143: Fix calling classloading transformers broken in > 7.0.70 > +b