svn commit: r1848225 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoaderBase.java WebappLoader.java

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r1848225 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoaderBase.java WebappLoader.java

remm
Author: remm
Date: Wed Dec  5 16:37:42 2018
New Revision: 1848225

URL: http://svn.apache.org/viewvc?rev=1848225&view=rev
Log:
Add i18n for the loader package.

Modified:
    tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties
    tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java
    tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java

Modified: tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties?rev=1848225&r1=1848224&r2=1848225&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties [UTF-8] (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties [UTF-8] Wed Dec  5 16:37:42 2018
@@ -46,6 +46,8 @@ webappClassLoader.loadedByThisOrChildFai
 webappClassLoader.readError=Resource read error: Could not load [{0}].
 webappClassLoader.removeTransformer=Removed class file transformer [{0}] from web application [{1}].
 webappClassLoader.resourceModified=Resource [{0}] has been modified. The last modified time was [{1}] and is now [{2}]
+webappClassLoader.restrictedPackage=Security violation, attempt to use restricted class [{0}]
+webappClassLoader.securityException=Security exception trying to find class [{0}] in findClassInternal [{1}]
 webappClassLoader.stackTrace=The web application [{0}] appears to have started a thread named [{1}] but has failed to stop it. This is very likely to create a memory leak. Stack trace of thread:{2}
 webappClassLoader.stackTraceRequestThread=The web application [{0}] is still processing a request that has yet to finish. This is very likely to create a memory leak. You can control the time allowed for requests to finish by using the unloadDelay attribute of the standard Context implementation. Stack trace of request processing thread:[{2}]
 webappClassLoader.stopThreadFail=Failed to terminate thread named [{0}] for web application [{1}]
@@ -64,8 +66,12 @@ webappLoader.copyFailure=Failed to copy
 webappLoader.deploy=Deploying class repositories to work directory [{0}]
 webappLoader.jarDeploy=Deploy JAR [{0}] to [{1}]
 webappLoader.mkdirFailure=Failed to create destination directory to copy resources
+webappLoader.noResources=No resources found for context [{0}]
 webappLoader.readFailure=Unable to read resource [{0}]
 webappLoader.reloadable=Cannot set reloadable property to [{0}]
 webappLoader.setContext.ise=Setting the Context is not permitted while the loader is started.
+webappLoader.startError=Error starting the loader
 webappLoader.starting=Starting this Loader
+webappLoader.stopError=Error stopping the loader
 webappLoader.stopping=Stopping this Loader
+webappLoader.unknownClassLoader=Unknown class loader [{0}] of class [{1}]

Modified: tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java?rev=1848225&r1=1848224&r2=1848225&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java Wed Dec  5 16:37:42 2018
@@ -850,8 +850,8 @@ public abstract class WebappClassLoaderB
                     clazz = findClassInternal(name);
                 }
             } catch(AccessControlException ace) {
-                log.warn("WebappClassLoader.findClassInternal(" + name
-                        + ") security exception: " + ace.getMessage(), ace);
+                log.warn(sm.getString("webappClassLoader.securityException", name,
+                        ace.getMessage()), ace);
                 throw new ClassNotFoundException(name, ace);
             } catch (RuntimeException e) {
                 if (log.isTraceEnabled())
@@ -862,8 +862,8 @@ public abstract class WebappClassLoaderB
                 try {
                     clazz = super.findClass(name);
                 } catch(AccessControlException ace) {
-                    log.warn("WebappClassLoader.findClassInternal(" + name
-                            + ") security exception: " + ace.getMessage(), ace);
+                    log.warn(sm.getString("webappClassLoader.securityException", name,
+                            ace.getMessage()), ace);
                     throw new ClassNotFoundException(name, ace);
                 } catch (RuntimeException e) {
                     if (log.isTraceEnabled())
@@ -1278,8 +1278,7 @@ public abstract class WebappClassLoaderB
                     try {
                         securityManager.checkPackageAccess(name.substring(0,i));
                     } catch (SecurityException se) {
-                        String error = "Security Violation, attempt to use " +
-                            "Restricted Class: " + name;
+                        String error = sm.getString("webappClassLoader.restrictedPackage", name);
                         log.info(error, se);
                         throw new ClassNotFoundException(error, se);
                     }

Modified: tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java?rev=1848225&r1=1848224&r2=1848225&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java Wed Dec  5 16:37:42 2018
@@ -62,6 +62,7 @@ import org.apache.tomcat.util.res.String
 public class WebappLoader extends LifecycleMBeanBase
     implements Loader, PropertyChangeListener {
 
+    private static final Log log = LogFactory.getLog(WebappLoader.class);
 
     // ----------------------------------------------------------- Constructors
 
@@ -377,7 +378,7 @@ public class WebappLoader extends Lifecy
             log.debug(sm.getString("webappLoader.starting"));
 
         if (context.getResources() == null) {
-            log.info("No resources for " + context);
+            log.info(sm.getString("webappLoader.noResources", context));
             setState(LifecycleState.STARTING);
             return;
         }
@@ -409,8 +410,7 @@ public class WebappLoader extends Lifecy
         } catch (Throwable t) {
             t = ExceptionUtils.unwrapInvocationTargetException(t);
             ExceptionUtils.handleThrowable(t);
-            log.error( "LifecycleException ", t );
-            throw new LifecycleException("start: ", t);
+            throw new LifecycleException(sm.getString("webappLoader.startError"), t);
         }
 
         setState(LifecycleState.STARTING);
@@ -455,7 +455,7 @@ public class WebappLoader extends Lifecy
                         context.getParent().getName() + ",context=" + contextName);
                 Registry.getRegistry(null, null).unregisterComponent(cloname);
             } catch (Exception e) {
-                log.error("LifecycleException ", e);
+                log.error(sm.getString("webappLoader.stopError"), e);
             }
         }
 
@@ -624,16 +624,12 @@ public class WebappLoader extends Lifecy
             }
             return false;
         } else {
-            log.info( "Unknown loader " + loader + " " + loader.getClass());
+            log.info(sm.getString("webappLoader.unknownClassLoader", loader, loader.getClass()));
             return false;
         }
         return true;
     }
 
-
-    private static final Log log = LogFactory.getLog(WebappLoader.class);
-
-
     @Override
     protected String getDomainInternal() {
         return context.getDomain();



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1848225 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoaderBase.java WebappLoader.java

remm
On Wed, Dec 5, 2018 at 5:37 PM <[hidden email]> wrote:

> Author: remm
> Date: Wed Dec  5 16:37:42 2018
> New Revision: 1848225
>
> URL: http://svn.apache.org/viewvc?rev=1848225&view=rev
> Log:
> Add i18n for the loader package.
>
> I have at least 350 more strings to add.

I don't know what to do with:
- the bootstrap.jar strings: nothing ?
- the "upstream" strings: dbcp, pool, bcel, fileupload

Rémy
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1848225 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoaderBase.java WebappLoader.java

markt
On 05/12/2018 20:03, Rémy Maucherat wrote:

> On Wed, Dec 5, 2018 at 5:37 PM <[hidden email]> wrote:
>
>> Author: remm
>> Date: Wed Dec  5 16:37:42 2018
>> New Revision: 1848225
>>
>> URL: http://svn.apache.org/viewvc?rev=1848225&view=rev
>> Log:
>> Add i18n for the loader package.
>>
>> I have at least 350 more strings to add.
>
> I don't know what to do with:
> - the bootstrap.jar strings: nothing ?

Refactor StringManager to JULI? It is somewhat logging related so that
doesn't seem completely crazy.

> - the "upstream" strings: dbcp, pool, bcel, fileupload

We could 'fix' them. It would mean the code diverging further from the
original which will make syncing harder. Roughly how many Strings are we
talking about here?

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1848225 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoaderBase.java WebappLoader.java

remm
On Wed, Dec 5, 2018 at 9:17 PM Mark Thomas <[hidden email]> wrote:

> On 05/12/2018 20:03, Rémy Maucherat wrote:
> > On Wed, Dec 5, 2018 at 5:37 PM <[hidden email]> wrote:
> >
> >> Author: remm
> >> Date: Wed Dec  5 16:37:42 2018
> >> New Revision: 1848225
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1848225&view=rev
> >> Log:
> >> Add i18n for the loader package.
> >>
> >> I have at least 350 more strings to add.
> >
> > I don't know what to do with:
> > - the bootstrap.jar strings: nothing ?
>
> Refactor StringManager to JULI? It is somewhat logging related so that
> doesn't seem completely crazy.
>

Ok, why not.

>
> > - the "upstream" strings: dbcp, pool, bcel, fileupload
>
> We could 'fix' them. It would mean the code diverging further from the
> original which will make syncing harder. Roughly how many Strings are we
> talking about here?
>

About 200.

Rémy
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1848225 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoaderBase.java WebappLoader.java

markt
On 05/12/2018 20:40, Rémy Maucherat wrote:

> On Wed, Dec 5, 2018 at 9:17 PM Mark Thomas <[hidden email]> wrote:
>
>> On 05/12/2018 20:03, Rémy Maucherat wrote:
>>> On Wed, Dec 5, 2018 at 5:37 PM <[hidden email]> wrote:
>>>
>>>> Author: remm
>>>> Date: Wed Dec  5 16:37:42 2018
>>>> New Revision: 1848225
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1848225&view=rev
>>>> Log:
>>>> Add i18n for the loader package.
>>>>
>>>> I have at least 350 more strings to add.
>>>
>>> I don't know what to do with:
>>> - the bootstrap.jar strings: nothing ?
>>
>> Refactor StringManager to JULI? It is somewhat logging related so that
>> doesn't seem completely crazy.
>>
>
> Ok, why not.

Apart from the fact it would mean touching almost every class in Tomcat?

Seriously, the more I think about it the more I like it. Move the res
package from o.a.tomcat.util.res o.a.juli.res and deprecate the old one
along with the copy in tribes. And then remove the old ones in 10.x

>>> - the "upstream" strings: dbcp, pool, bcel, fileupload
>>
>> We could 'fix' them. It would mean the code diverging further from the
>> original which will make syncing harder. Roughly how many Strings are we
>> talking about here?
>>
>
> About 200.

Hmm. DBCP2 already has simple l10n support along the lines of:

private static final ResourceBundle messages = ResourceBundle.getBundle(
        Utils.class.getPackage().getName() + ".LocalStrings");

That could be extended. Every ASF committer has commit access to Commons
so I guess it could just be fixed at source. I'll volunteer to merge the
changes back into Tomcat.

Pool2 doesn't use a ResourceBundle but I don't see why it couldn't. Same
for FileUpload.

BCEL code is so far removed from Commons BCEL I'd probably just fix it
locally.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1848225 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoaderBase.java WebappLoader.java

remm
On Thu, Dec 6, 2018 at 2:42 PM Mark Thomas <[hidden email]> wrote:

> > Ok, why not.
>
> Apart from the fact it would mean touching almost every class in Tomcat?
>
> Seriously, the more I think about it the more I like it. Move the res
> package from o.a.tomcat.util.res o.a.juli.res and deprecate the old one
> along with the copy in tribes. And then remove the old ones in 10.x
>

There's another problem with that startup package where the bundles of the
actual bootstrap are shared with the rest (ContextConfig, etc). Also, is it
really good to start shipping plenty of resources in a "small" bootstrap ?
Open questions.

>
> >>> - the "upstream" strings: dbcp, pool, bcel, fileupload
> >>
> >> We could 'fix' them. It would mean the code diverging further from the
> >> original which will make syncing harder. Roughly how many Strings are we
> >> talking about here?
> >>
> >
> > About 200.
>
> Hmm. DBCP2 already has simple l10n support along the lines of:
>
> private static final ResourceBundle messages = ResourceBundle.getBundle(
>         Utils.class.getPackage().getName() + ".LocalStrings");
>
> That could be extended. Every ASF committer has commit access to Commons
> so I guess it could just be fixed at source. I'll volunteer to merge the
> changes back into Tomcat.
>
> Pool2 doesn't use a ResourceBundle but I don't see why it couldn't. Same
> for FileUpload.
>
> BCEL code is so far removed from Commons BCEL I'd probably just fix it
> locally.
>

Ok. OTOH about DBCP it's surprising there's something because in other
places you find this kind of code which means nobody cares at all:
stringBuilder.append(NUPROP_WARNTEXT.get(propertyName)).append(" You have
set value of \"")
                            .append(propertyValue).append("\" for
\"").append(propertyName)
                            .append("\" property, which is being ignored.");

For sure fixing this will be done last ;)

Rémy
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1848225 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoaderBase.java WebappLoader.java

markt
On 06/12/2018 14:08, Rémy Maucherat wrote:

> On Thu, Dec 6, 2018 at 2:42 PM Mark Thomas <[hidden email]> wrote:
>
>>> Ok, why not.
>>
>> Apart from the fact it would mean touching almost every class in Tomcat?
>>
>> Seriously, the more I think about it the more I like it. Move the res
>> package from o.a.tomcat.util.res o.a.juli.res and deprecate the old one
>> along with the copy in tribes. And then remove the old ones in 10.x
>>
>
> There's another problem with that startup package where the bundles of the
> actual bootstrap are shared with the rest (ContextConfig, etc). Also, is it
> really good to start shipping plenty of resources in a "small" bootstrap ?
> Open questions.

Indeed. I don't think we are under any pressure to find an answer in a
hurry. We can keep discussing until we have a solution that everyone is
happy with.

Should we move the bootstrap classes to a separate package?

That won't address the issues of how to package the resource files for
that package.

Size shouldn't be too much of an issue with the scope limited to just
bootstrap. But if they are all in a single JAR disabling a language by
deleting the JAR won't work for the bootstrap resources. I don't think
per language bootstrap resource JARs are the solution.


>>>>> - the "upstream" strings: dbcp, pool, bcel, fileupload
>>>>
>>>> We could 'fix' them. It would mean the code diverging further from the
>>>> original which will make syncing harder. Roughly how many Strings are we
>>>> talking about here?
>>>>
>>>
>>> About 200.
>>
>> Hmm. DBCP2 already has simple l10n support along the lines of:
>>
>> private static final ResourceBundle messages = ResourceBundle.getBundle(
>>         Utils.class.getPackage().getName() + ".LocalStrings");
>>
>> That could be extended. Every ASF committer has commit access to Commons
>> so I guess it could just be fixed at source. I'll volunteer to merge the
>> changes back into Tomcat.
>>
>> Pool2 doesn't use a ResourceBundle but I don't see why it couldn't. Same
>> for FileUpload.
>>
>> BCEL code is so far removed from Commons BCEL I'd probably just fix it
>> locally.
>>
>
> Ok. OTOH about DBCP it's surprising there's something because in other
> places you find this kind of code which means nobody cares at all:
> stringBuilder.append(NUPROP_WARNTEXT.get(propertyName)).append(" You have
> set value of \"")
>                             .append(propertyValue).append("\" for
> \"").append(propertyName)
>                             .append("\" property, which is being ignored.");
>
> For sure fixing this will be done last ;)

Yep.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1848225 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoaderBase.java WebappLoader.java

remm
On Fri, Dec 7, 2018 at 6:53 PM Mark Thomas <[hidden email]> wrote:

> On 06/12/2018 14:08, Rémy Maucherat wrote:
> > On Thu, Dec 6, 2018 at 2:42 PM Mark Thomas <[hidden email]> wrote:
> >
> >>> Ok, why not.
> >>
> >> Apart from the fact it would mean touching almost every class in Tomcat?
> >>
> >> Seriously, the more I think about it the more I like it. Move the res
> >> package from o.a.tomcat.util.res o.a.juli.res and deprecate the old one
> >> along with the copy in tribes. And then remove the old ones in 10.x
> >>
> >
> > There's another problem with that startup package where the bundles of
> the
> > actual bootstrap are shared with the rest (ContextConfig, etc). Also, is
> it
> > really good to start shipping plenty of resources in a "small" bootstrap
> ?
> > Open questions.
>
> Indeed. I don't think we are under any pressure to find an answer in a
> hurry. We can keep discussing until we have a solution that everyone is
> happy with.
>
> Should we move the bootstrap classes to a separate package?
>
> That won't address the issues of how to package the resource files for
> that package.
>
> Size shouldn't be too much of an issue with the scope limited to just
> bootstrap. But if they are all in a single JAR disabling a language by
> deleting the JAR won't work for the bootstrap resources. I don't think
> per language bootstrap resource JARs are the solution.
>

As a refinement, the bootstrap resources could go in a subpackage of
startup, that avoids any API change issue. For packaging, I would put them
in bootstrap.jar, there's no real way to do otherwise. Anyway, it's not
really important, there's nothing very interesting in those strings, so
maybe it's only a rhetorical issue and nothing will be done.

I still have about 200 strings to go for the core Tomcat, excluding DBCP
and others.

Rémy
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1848225 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoaderBase.java WebappLoader.java

Konstantin Kolinko
In reply to this post by markt
чт, 6 дек. 2018 г. в 16:42, Mark Thomas <[hidden email]>:

>
> On 05/12/2018 20:40, Rémy Maucherat wrote:
> > On Wed, Dec 5, 2018 at 9:17 PM Mark Thomas <[hidden email]> wrote:
> >
> >> On 05/12/2018 20:03, Rémy Maucherat wrote:
> >>> On Wed, Dec 5, 2018 at 5:37 PM <[hidden email]> wrote:
> >>>
> >>>> Author: remm
> >>>> Date: Wed Dec  5 16:37:42 2018
> >>>> New Revision: 1848225
> >>>>
> >>>> URL: http://svn.apache.org/viewvc?rev=1848225&view=rev
> >>>> Log:
> >>>> Add i18n for the loader package.
> >>>>
> >>>> I have at least 350 more strings to add.
> >>>
> >>> I don't know what to do with:
> >>> - the bootstrap.jar strings: nothing ?
> >>
> >> Refactor StringManager to JULI? It is somewhat logging related so that
> >> doesn't seem completely crazy.
> >>
> >
> > Ok, why not.
>
> Apart from the fact it would mean touching almost every class in Tomcat?
>
> Seriously, the more I think about it the more I like it. Move the res
> package from o.a.tomcat.util.res o.a.juli.res and deprecate the old one
> along with the copy in tribes. And then remove the old ones in 10.x

JULI and JDBC pool can be used standalone, without Tomcat. It does not
make much sense to localize those.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]