Skip to content

Commit

Permalink
Simplify
Browse files Browse the repository at this point in the history
Also avoid falling back always to getSource().getResource since it will
produce a misleading error when actually using the prefix (which looks
like a uri).
  • Loading branch information
rmaucher committed Jun 29, 2023
1 parent 493a6fe commit 6391070
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 27 deletions.
2 changes: 2 additions & 0 deletions java/org/apache/catalina/Context.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.catalina;

import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.net.URISyntaxException;
Expand Down Expand Up @@ -1997,6 +1998,7 @@ default Resource findConfigFileResource(String name) throws IOException {
stream.close();

This comment has been minimized.

Copy link
@michael-o

michael-o Jun 29, 2023

Member

Now this be a fallthrough to FNFE which isn't right. It should be: throw new IOException(sm.getString("catalinaConfigurationSource.cannotObtainURL", name), e);

}
}
throw new FileNotFoundException(name);

This comment has been minimized.

Copy link
@michael-o

michael-o Jun 29, 2023

Member

This is still incomplete, the actual message is missing. See: throw new FileNotFoundException(sm.getString("classpathUrlStreamHandler.notFound", u));

}
return ConfigFileLoader.getSource().getResource(name);
}
Expand Down
31 changes: 4 additions & 27 deletions java/org/apache/catalina/core/PropertiesRoleMappingListener.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@
*/
package org.apache.catalina.core;

import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Properties;
Expand All @@ -30,6 +28,7 @@
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.file.ConfigFileLoader;
import org.apache.tomcat.util.file.ConfigurationSource.Resource;
import org.apache.tomcat.util.res.StringManager;

/**
Expand Down Expand Up @@ -107,32 +106,10 @@ public void lifecycleEvent(LifecycleEvent event) {
log.warn(sm.getString("listener.notContext", event.getLifecycle().getClass().getSimpleName()));
return;
}
Context context = (Context) event.getLifecycle();

InputStream is;
if (roleMappingFile.startsWith(WEBAPP_PROTOCOL)) {
String path = roleMappingFile.substring(WEBAPP_PROTOCOL.length());
is = context.getServletContext().getResourceAsStream(path);
} else {
try {
is = ConfigFileLoader.getSource().getResource(roleMappingFile).getInputStream();
} catch (FileNotFoundException e1) {
is = null;
} catch (IOException e2) {
throw new IllegalStateException(
sm.getString("propertiesRoleMappingListener.roleMappingFileFail", roleMappingFile), e2);
}
}

if (is == null) {
throw new IllegalStateException(
sm.getString("propertiesRoleMappingListener.roleMappingFileNotFound", roleMappingFile));
}

Properties props = new Properties();

try (InputStream _is = is) {
props.load(_is);
Context context = (Context) event.getLifecycle();
try (Resource resource = context.findConfigFileResource(roleMappingFile)) {
props.load(resource.getInputStream());
} catch (IOException e) {

This comment has been minimized.

Copy link
@michael-o

michael-o Jun 29, 2023

Member

Since you don't differentiate between FNFE and IOE the log message quality has been reduced

throw new IllegalStateException(
sm.getString("propertiesRoleMappingListener.roleMappingFileFail", roleMappingFile), e);
Expand Down

0 comments on commit 6391070

Please sign in to comment.