ASF Bugzilla – Attachment 33811 Details for
Bug 52952
Improve ExtensionValidator handling for embedded scenarios
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-52952.patch (text/plain), 24.60 KB, created by
Abdessamed MANSOURI
on 2016-04-27 13:35:42 UTC
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Abdessamed MANSOURI
Created:
2016-04-27 13:35:42 UTC
Size:
24.60 KB
patch
obsolete
>Index: java/org/apache/catalina/core/StandardServer.java >=================================================================== >--- java/org/apache/catalina/core/StandardServer.java (revision 1740624) >+++ java/org/apache/catalina/core/StandardServer.java (working copy) >@@ -470,7 +470,7 @@ > // This should never happen but bug 56684 suggests that > // it does. > log.warn(sm.getString("standardServer.accept.timeout", >- Long.valueOf(System.currentTimeMillis() - acceptStartTime)), ste); >+ System.currentTimeMillis() - acceptStartTime), ste); > continue; > } catch (AccessControlException ace) { > log.warn("StandardServer.accept security exception: " >@@ -498,7 +498,6 @@ > ch = stream.read(); > } catch (IOException e) { > log.warn("StandardServer.await: read: ", e); >- ch = -1; > } > if (ch < 32) // Control character or EOF terminates loop > break; >@@ -555,9 +554,9 @@ > return (null); > } > synchronized (servicesLock) { >- for (int i = 0; i < services.length; i++) { >- if (name.equals(services[i].getName())) { >- return (services[i]); >+ for (Service service : services) { >+ if (name.equals(service.getName())) { >+ return (service); > } > } > } >@@ -687,12 +686,7 @@ > */ > @Override > public String toString() { >- >- StringBuilder sb = new StringBuilder("StandardServer["); >- sb.append(getPort()); >- sb.append("]"); >- return (sb.toString()); >- >+ return "StandardServer[" + getPort() + "]"; > } > > >@@ -787,8 +781,8 @@ > > // Start our defined Services > synchronized (servicesLock) { >- for (int i = 0; i < services.length; i++) { >- services[i].start(); >+ for (Service service : services) { >+ service.start(); > } > } > } >@@ -808,8 +802,8 @@ > fireLifecycleEvent(CONFIGURE_STOP_EVENT, null); > > // Stop our defined Services >- for (int i = 0; i < services.length; i++) { >- services[i].stop(); >+ for (Service service : services) { >+ service.stop(); > } > > globalNamingResources.stop(); >@@ -857,10 +851,8 @@ > f.getName().endsWith(".jar")) { > ExtensionValidator.addSystemResource(f); > } >- } catch (URISyntaxException e) { >+ } catch (URISyntaxException | IOException e) { > // Ignore >- } catch (IOException e) { >- // Ignore > } > } > } >@@ -869,8 +861,8 @@ > } > } > // Initialize our defined Services >- for (int i = 0; i < services.length; i++) { >- services[i].init(); >+ for (Service service : services) { >+ service.init(); > } > } > >@@ -877,8 +869,8 @@ > @Override > protected void destroyInternal() throws LifecycleException { > // Destroy our defined Services >- for (int i = 0; i < services.length; i++) { >- services[i].destroy(); >+ for (Service service : services) { >+ service.destroy(); > } > > globalNamingResources.destroy(); >Index: java/org/apache/catalina/util/Extension.java >=================================================================== >--- java/org/apache/catalina/util/Extension.java (revision 1740624) >+++ java/org/apache/catalina/util/Extension.java (working copy) >@@ -19,6 +19,7 @@ > package org.apache.catalina.util; > > >+import java.util.Objects; > import java.util.StringTokenizer; > > >@@ -44,7 +45,6 @@ > */ > public final class Extension { > >- > // ------------------------------------------------------------- Properties > > >@@ -51,7 +51,7 @@ > /** > * The name of the optional package being made available, or required. > */ >- private String extensionName = null; >+ private String extensionName; > > > public String getExtensionName() { >@@ -66,7 +66,7 @@ > * The URL from which the most recent version of this optional package > * can be obtained if it is not already installed. > */ >- private String implementationURL = null; >+ private String implementationURL; > > public String getImplementationURL() { > return (this.implementationURL); >@@ -81,7 +81,7 @@ > * The name of the company or organization that produced this > * implementation of this optional package. > */ >- private String implementationVendor = null; >+ private String implementationVendor; > > public String getImplementationVendor() { > return (this.implementationVendor); >@@ -96,7 +96,7 @@ > * The unique identifier of the company that produced the optional > * package contained in this JAR file. > */ >- private String implementationVendorId = null; >+ private String implementationVendorId; > > public String getImplementationVendorId() { > return (this.implementationVendorId); >@@ -111,7 +111,7 @@ > * The version number (dotted decimal notation) for this implementation > * of the optional package. > */ >- private String implementationVersion = null; >+ private String implementationVersion; > > public String getImplementationVersion() { > return (this.implementationVersion); >@@ -126,7 +126,7 @@ > * The name of the company or organization that originated the > * specification to which this optional package conforms. > */ >- private String specificationVendor = null; >+ private String specificationVendor; > > public String getSpecificationVendor() { > return (this.specificationVendor); >@@ -141,7 +141,7 @@ > * The version number (dotted decimal notation) of the specification > * to which this optional package conforms. > */ >- private String specificationVersion = null; >+ private String specificationVersion; > > public String getSpecificationVersion() { > return (this.specificationVersion); >@@ -153,10 +153,11 @@ > > > /** >+ * mark this extension as fulfilled > * fulfilled is true if all the required extension dependencies have been > * satisfied > */ >- private boolean fulfilled = false; >+ private boolean fulfilled; > > public void setFulfilled(boolean fulfilled) { > this.fulfilled = fulfilled; >@@ -248,7 +249,26 @@ > > } > >+ @Override >+ public boolean equals(Object o) { >+ if (this == o) return true; >+ if (!(o instanceof Extension)) return false; >+ Extension extension = (Extension) o; >+ return Objects.equals(extensionName, extension.extensionName) && >+ Objects.equals(implementationURL, extension.implementationURL) && >+ Objects.equals(implementationVendor, extension.implementationVendor) && >+ Objects.equals(implementationVendorId, extension.implementationVendorId) && >+ Objects.equals(implementationVersion, extension.implementationVersion) && >+ Objects.equals(specificationVendor, extension.specificationVendor) && >+ Objects.equals(specificationVersion, extension.specificationVersion); >+ } > >+ @Override >+ public int hashCode() { >+ return Objects.hash(extensionName, implementationURL, implementationVendor, implementationVendorId, >+ implementationVersion, specificationVendor, specificationVersion); >+ } >+ > // -------------------------------------------------------- Private Methods > > >@@ -272,8 +292,8 @@ > > StringTokenizer fTok = new StringTokenizer(first, ".", true); > StringTokenizer sTok = new StringTokenizer(second, ".", true); >- int fVersion = 0; >- int sVersion = 0; >+ int fVersion; >+ int sVersion; > while (fTok.hasMoreTokens() || sTok.hasMoreTokens()) { > if (fTok.hasMoreTokens()) > fVersion = Integer.parseInt(fTok.nextToken()); >Index: java/org/apache/catalina/util/ExtensionValidator.java >=================================================================== >--- java/org/apache/catalina/util/ExtensionValidator.java (revision 1740624) >+++ java/org/apache/catalina/util/ExtensionValidator.java (working copy) >@@ -20,9 +20,10 @@ > import java.io.FileInputStream; > import java.io.IOException; > import java.io.InputStream; >-import java.util.ArrayList; >-import java.util.Iterator; >+import java.util.HashSet; >+import java.util.List; > import java.util.Locale; >+import java.util.Set; > import java.util.StringTokenizer; > import java.util.jar.JarInputStream; > import java.util.jar.Manifest; >@@ -56,12 +57,13 @@ > private static final StringManager sm = > StringManager.getManager("org.apache.catalina.util"); > >- private static volatile ArrayList<Extension> containerAvailableExtensions = >- null; >- private static final ArrayList<ManifestResource> containerManifestResources = >- new ArrayList<>(); >+ private static final String MANIFEST_PATH = "/META-INF/MANIFEST.MF"; > >+ private static volatile Set<Extension> containerAvailableExtensions; >+ private static final Set<ManifestResource> containerManifestResources = >+ new HashSet<>(); > >+ > // ----------------------------------------------------- Static Initializer > > >@@ -129,16 +131,16 @@ > throws IOException { > > String appName = context.getName(); >- ArrayList<ManifestResource> appManifestResources = new ArrayList<>(); >+ Set<ManifestResource> appManifestResources = new HashSet<>(); > > // Web application manifest >- WebResource resource = resources.getResource("/META-INF/MANIFEST.MF"); >+ WebResource resource = resources.getResource(MANIFEST_PATH); > if (resource.isFile()) { > try (InputStream inputStream = resource.getInputStream()) { > Manifest manifest = new Manifest(inputStream); > ManifestResource mre = new ManifestResource > (sm.getString("extensionValidator.web-application-manifest"), >- manifest, ManifestResource.WAR); >+ manifest, ManifestResource.ResourceType.WAR); > appManifestResources.add(mre); > } > } >@@ -145,16 +147,15 @@ > > // Web application library manifests > WebResource[] manifestResources = >- resources.getClassLoaderResources("/META-INF/MANIFEST.MF"); >+ resources.getClassLoaderResources(MANIFEST_PATH); > for (WebResource manifestResource : manifestResources) { > if (manifestResource.isFile()) { > // Primarily used for error reporting > String jarName = manifestResource.getURL().toExternalForm(); >- Manifest jmanifest = null; > try (InputStream is = manifestResource.getInputStream()) { >- jmanifest = new Manifest(is); >+ Manifest jmanifest = new Manifest(is); > ManifestResource mre = new ManifestResource(jarName, >- jmanifest, ManifestResource.APPLICATION); >+ jmanifest, ManifestResource.ResourceType.APPLICATION); > appManifestResources.add(mre); > } > } >@@ -176,7 +177,7 @@ > Manifest manifest = getManifest(is); > if (manifest != null) { > ManifestResource mre = new ManifestResource(jarFile.getAbsolutePath(), manifest, >- ManifestResource.SYSTEM); >+ ManifestResource.ResourceType.SYSTEM); > containerManifestResources.add(mre); > } > } >@@ -187,7 +188,7 @@ > > > /** >- * Validates a <code>ArrayList</code> of <code>ManifestResource</code> >+ * Validates a <code>Set</code> of <code>ManifestResource</code> > * objects. This method requires an application name (which is the > * context root of the application at runtime). > * >@@ -206,15 +207,13 @@ > * @return true if manifest resource file requirements are met > */ > private static boolean validateManifestResources(String appName, >- ArrayList<ManifestResource> resources) { >+ Set<ManifestResource> resources) { > boolean passes = true; > int failureCount = 0; >- ArrayList<Extension> availableExtensions = null; >+ Set<Extension> availableExtensions = null; > >- Iterator<ManifestResource> it = resources.iterator(); >- while (it.hasNext()) { >- ManifestResource mre = it.next(); >- ArrayList<Extension> requiredList = mre.getRequiredExtensions(); >+ for (ManifestResource mre : resources) { >+ List<Extension> requiredList = mre.getRequiredExtensions(); > if (requiredList == null) { > continue; > } >@@ -228,19 +227,16 @@ > // yet > if (containerAvailableExtensions == null) { > containerAvailableExtensions >- = buildAvailableExtensionsList(containerManifestResources); >+ = buildAvailableExtensionsList(containerManifestResources); > } > > // iterate through the list of required extensions >- Iterator<Extension> rit = requiredList.iterator(); >- while (rit.hasNext()) { >+ for (Extension requiredExt : requiredList) { > boolean found = false; >- Extension requiredExt = rit.next(); >+ > // check the application itself for the extension > if (availableExtensions != null) { >- Iterator<Extension> ait = availableExtensions.iterator(); >- while (ait.hasNext()) { >- Extension targetExt = ait.next(); >+ for (Extension targetExt : availableExtensions) { > if (targetExt.isCompatibleWith(requiredExt)) { > requiredExt.setFulfilled(true); > found = true; >@@ -248,12 +244,10 @@ > } > } > } >+ > // check the container level list for the extension > if (!found && containerAvailableExtensions != null) { >- Iterator<Extension> cit = >- containerAvailableExtensions.iterator(); >- while (cit.hasNext()) { >- Extension targetExt = cit.next(); >+ for (Extension targetExt : containerAvailableExtensions) { > if (targetExt.isCompatibleWith(requiredExt)) { > requiredExt.setFulfilled(true); > found = true; >@@ -261,12 +255,13 @@ > } > } > } >+ > if (!found) { > // Failure > log.info(sm.getString( >- "extensionValidator.extension-not-found-error", >- appName, mre.getResourceName(), >- requiredExt.getExtensionName())); >+ "extensionValidator.extension-not-found-error", >+ appName, mre.getResourceName(), >+ requiredExt.getExtensionName())); > passes = false; > failureCount++; > } >@@ -299,25 +294,19 @@ > * > * @return HashMap Map of available extensions > */ >- private static ArrayList<Extension> buildAvailableExtensionsList( >- ArrayList<ManifestResource> resources) { >+ private static Set<Extension> buildAvailableExtensionsList( >+ Set<ManifestResource> resources) { > >- ArrayList<Extension> availableList = null; >+ Set<Extension> availableList = null; > >- Iterator<ManifestResource> it = resources.iterator(); >- while (it.hasNext()) { >- ManifestResource mre = it.next(); >- ArrayList<Extension> list = mre.getAvailableExtensions(); >+ for (ManifestResource mre : resources) { >+ List<Extension> list = mre.getAvailableExtensions(); > if (list != null) { >- Iterator<Extension> values = list.iterator(); >- while (values.hasNext()) { >- Extension ext = values.next(); >+ for (Extension ext : list) { > if (availableList == null) { >- availableList = new ArrayList<>(); >- availableList.add(ext); >- } else { >- availableList.add(ext); >+ availableList = new HashSet<>(); > } >+ availableList.add(ext); > } > } > } >@@ -332,7 +321,7 @@ > * @return The WAR's or JAR's manifest > */ > private static Manifest getManifest(InputStream inStream) throws IOException { >- Manifest manifest = null; >+ Manifest manifest; > try (JarInputStream jin = new JarInputStream(inStream)) { > manifest = jin.getManifest(); > } >@@ -359,15 +348,16 @@ > if (files == null) { > continue; > } >- for (int i = 0; i < files.length; i++) { >- if (files[i].getName().toLowerCase(Locale.ENGLISH).endsWith(".jar") && >- files[i].isFile()) { >+ >+ for (File file : files) { >+ if (file.getName().toLowerCase(Locale.ENGLISH).endsWith(".jar") && >+ file.isFile()) { > try { >- addSystemResource(files[i]); >+ addSystemResource(file); > } catch (IOException e) { > log.error >- (sm.getString >- ("extensionValidator.failload", files[i]), e); >+ (sm.getString >+ ("extensionValidator.failload", file), e); > } > } > } >Index: java/org/apache/catalina/util/ManifestResource.java >=================================================================== >--- java/org/apache/catalina/util/ManifestResource.java (revision 1740624) >+++ java/org/apache/catalina/util/ManifestResource.java (working copy) >@@ -16,8 +16,9 @@ > */ > package org.apache.catalina.util; > >-import java.util.ArrayList; >-import java.util.Iterator; >+import java.nio.file.Path; >+import java.nio.file.Paths; >+import java.util.*; > import java.util.jar.Attributes; > import java.util.jar.Manifest; > >@@ -32,19 +33,21 @@ > > // ------------------------------------------------------------- Properties > >- // These are the resource types for determining effect error messages >- public static final int SYSTEM = 1; >- public static final int WAR = 2; >- public static final int APPLICATION = 3; >+ // This enum is the resource types for determining effect error messages >+ public enum ResourceType { >+ SYSTEM, >+ WAR, >+ APPLICATION >+ } > >- private ArrayList<Extension> availableExtensions = null; >- private ArrayList<Extension> requiredExtensions = null; >+ private List<Extension> availableExtensions; >+ private List<Extension> requiredExtensions; > > private final String resourceName; >- private final int resourceType; >+ private final ResourceType resourceType; > > public ManifestResource(String resourceName, Manifest manifest, >- int resourceType) { >+ ResourceType resourceType) { > this.resourceName = resourceName; > this.resourceType = resourceType; > processManifest(manifest); >@@ -62,19 +65,19 @@ > /** > * Gets the list of available extensions > * >- * @return List of available extensions >+ * @return Unmodifiable list of available extensions > */ >- public ArrayList<Extension> getAvailableExtensions() { >- return availableExtensions; >+ public List<Extension> getAvailableExtensions() { >+ return Collections.unmodifiableList(availableExtensions); > } > > /** > * Gets the list of required extensions > * >- * @return List of required extensions >+ * @return Unmodifiable list of required extensions > */ >- public ArrayList<Extension> getRequiredExtensions() { >- return requiredExtensions; >+ public List<Extension> getRequiredExtensions() { >+ return Collections.unmodifiableList(requiredExtensions); > } > > // --------------------------------------------------------- Public Methods >@@ -107,11 +110,10 @@ > if (requiredExtensions == null) { > return true; > } >- Iterator<Extension> it = requiredExtensions.iterator(); >- while (it.hasNext()) { >- Extension ext = it.next(); >+ for(Extension ext : requiredExtensions){ > if (!ext.isFulfilled()) return false; > } >+ > return true; > } > >@@ -118,24 +120,33 @@ > @Override > public String toString() { > >- StringBuilder sb = new StringBuilder("ManifestResource["); >- sb.append(resourceName); >+ return "ManifestResource[" + resourceName + >+ ", isFulfilled=" + >+ isFulfilled() + >+ ", requiredExtensionCount =" + >+ getRequiredExtensionCount() + >+ ", availableExtensionCount=" + >+ getAvailableExtensionCount() + >+ ", resourceType=" + >+ resourceType + >+ "]"; >+ } > >- sb.append(", isFulfilled="); >- sb.append(isFulfilled() +""); >- sb.append(", requiredExtensionCount ="); >- sb.append(getRequiredExtensionCount()); >- sb.append(", availableExtensionCount="); >- sb.append(getAvailableExtensionCount()); >- switch (resourceType) { >- case SYSTEM : sb.append(", resourceType=SYSTEM"); break; >- case WAR : sb.append(", resourceType=WAR"); break; >- case APPLICATION : sb.append(", resourceType=APPLICATION"); break; >- } >- sb.append("]"); >- return (sb.toString()); >+ @Override >+ public boolean equals(Object o) { >+ if (this == o) return true; >+ if (!(o instanceof ManifestResource)) return false; >+ ManifestResource that = (ManifestResource) o; >+ return Objects.equals(availableExtensions, that.availableExtensions) && >+ Objects.equals(requiredExtensions, that.requiredExtensions) && >+ Objects.equals(resourceName, that.resourceName) && >+ resourceType == that.resourceType; > } > >+ @Override >+ public int hashCode() { >+ return Objects.hash(availableExtensions, requiredExtensions, resourceName, resourceType); >+ } > > // -------------------------------------------------------- Private Methods > >@@ -154,7 +165,7 @@ > * @return List of required extensions, or null if the application > * does not require any extensions > */ >- private ArrayList<Extension> getRequiredExtensions(Manifest manifest) { >+ private List<Extension> getRequiredExtensions(Manifest manifest) { > > Attributes attributes = manifest.getMainAttributes(); > String names = attributes.getValue("Extension-List"); >@@ -161,7 +172,7 @@ > if (names == null) > return null; > >- ArrayList<Extension> extensionList = new ArrayList<>(); >+ List<Extension> extensionList = new ArrayList<>(); > names += " "; > > while (true) { >@@ -201,7 +212,7 @@ > * @return List of available extensions, or null if the web application > * does not bundle any extensions > */ >- private ArrayList<Extension> getAvailableExtensions(Manifest manifest) { >+ private List<Extension> getAvailableExtensions(Manifest manifest) { > > Attributes attributes = manifest.getMainAttributes(); > String name = attributes.getValue("Extension-Name"); >@@ -208,7 +219,7 @@ > if (name == null) > return null; > >- ArrayList<Extension> extensionList = new ArrayList<>(); >+ List<Extension> extensionList = new ArrayList<>(); > > Extension extension = new Extension(); > extension.setExtensionName(name); >@@ -227,5 +238,4 @@ > > return extensionList; > } >- > }
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 52952
:
33811
|
33813
|
33858