Bugzilla#53685 - AFP charactersets are cached using more unique keys

git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@1371639 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/java/org/apache/fop/afp/fonts/CharacterSetBuilder.java b/src/java/org/apache/fop/afp/fonts/CharacterSetBuilder.java
index 2aa5a9eeab13a0e33eac8941aabaa54cc887ea6b..2565942b5524d389f9745398e6eb69fd2ab54551 100644 (file)
--- a/src/java/org/apache/fop/afp/fonts/CharacterSetBuilder.java
+++ b/src/java/org/apache/fop/afp/fonts/CharacterSetBuilder.java
@@ -236,9 +236,9 @@ public abstract class CharacterSetBuilder {
             CharacterSetType charsetType, AFPResourceAccessor accessor, AFPEventProducer eventProducer)
             throws IOException {
         // check for cached version of the characterset
-        String descriptor = characterSetName + "_" + encoding + "_" + codePageName;
-        CharacterSet characterSet = (CharacterSet) characterSetsCache.get(descriptor);
-
+        URI charSetURI = accessor.resolveURI(characterSetName);
+        String cacheKey = charSetURI.toASCIIString() + "_" + characterSetName + "_" + codePageName;
+        CharacterSet characterSet = (CharacterSet) characterSetsCache.get(cacheKey);
         if (characterSet != null) {
             return characterSet;
         }
@@ -257,6 +257,8 @@ public abstract class CharacterSetBuilder {
              * chracter global identifier.
              */
             Map<String, String> codePage;
+            // TODO: This could have performance implications if several threads want to use the
+            // codePagesCache to retrieve different codepages.
             synchronized (codePagesCache) {
                 codePage = codePagesCache.get(codePageName);
 
@@ -308,7 +310,7 @@ public abstract class CharacterSetBuilder {
         } finally {
             closeInputStream(inputStream);
         }
-        characterSetsCache.put(descriptor, characterSet);
+        characterSetsCache.put(cacheKey, characterSet);
         return characterSet;
     }
 

diff --git a/src/java/org/apache/fop/afp/modca/IncludedResourceObject.java b/src/java/org/apache/fop/afp/modca/IncludedResourceObject.java
index 9ab84a64018cf32f503e43db9a2dfaa303eaafc5..8bfcf4624ec7470333ba00fe6f79b4823e9a1f5d 100644 (file)
--- a/src/java/org/apache/fop/afp/modca/IncludedResourceObject.java
+++ b/src/java/org/apache/fop/afp/modca/IncludedResourceObject.java
@@ -34,7 +34,7 @@ import org.apache.fop.afp.util.AFPResourceUtil;
  */
 public class IncludedResourceObject extends AbstractNamedAFPObject {
 
-    private AFPResourceAccessor resourceAccessor;
+    private final AFPResourceAccessor resourceAccessor;
     private URI uri;
 
     /**

diff --git a/src/java/org/apache/fop/afp/util/AFPResourceAccessor.java b/src/java/org/apache/fop/afp/util/AFPResourceAccessor.java
index 1663bbebe365c7464f9570be688b9d6e45b593c0..46b09be21ed7e6d2a5125fe83aecb7a369b0f57f 100644 (file)
--- a/src/java/org/apache/fop/afp/util/AFPResourceAccessor.java
+++ b/src/java/org/apache/fop/afp/util/AFPResourceAccessor.java
@@ -24,6 +24,9 @@ import java.io.InputStream;
 import java.net.URI;
 import java.net.URISyntaxException;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
 import org.apache.fop.apps.io.InternalResourceResolver;
 
 /**
@@ -31,8 +34,11 @@ import org.apache.fop.apps.io.InternalResourceResolver;
  */
 public final class AFPResourceAccessor {
 
+    private static final Log log = LogFactory.getLog(AFPResourceAccessor.class);
+
     private final InternalResourceResolver resourceResolver;
-    private final String baseURI;
+    private final URI baseURI;
+    private final URIResolver uriResolver;
 
     /**
      * Constructor for resource to be accessed via the {@link org.apache.fop.apps.FOUserAgent}. This
@@ -45,7 +51,23 @@ public final class AFPResourceAccessor {
      */
     public AFPResourceAccessor(InternalResourceResolver resourceResolver, String baseURI) {
         this.resourceResolver = resourceResolver;
-        this.baseURI = baseURI;
+        URI actualBaseURI = null;
+        URIResolver uriResolver;
+        if (baseURI == null) {
+            actualBaseURI = null;
+            uriResolver = new NullBaseURIResolver();
+        } else {
+            try {
+                actualBaseURI = InternalResourceResolver.getBaseURI(baseURI);
+                uriResolver = new BaseURIResolver();
+            } catch (URISyntaxException use) {
+                log.error("The URI given \"" + baseURI + "\" is invalid: " + use.getMessage());
+                actualBaseURI = null;
+                uriResolver = new NullBaseURIResolver();
+            }
+        }
+        this.baseURI = actualBaseURI;
+        this.uriResolver = uriResolver;
     }
 
     /**
@@ -57,18 +79,6 @@ public final class AFPResourceAccessor {
         this(resourceResolver, null);
     }
 
-    private URI getResourceURI(URI uri) {
-        if (baseURI == null) {
-            return uri;
-        }
-        try {
-            URI baseURI = InternalResourceResolver.getBaseURI(this.baseURI);
-            return baseURI.resolve(uri);
-        } catch (URISyntaxException use) {
-            return uri;
-        }
-    }
-
     /**
      * Creates an {@link InputStream} given a URI.
      *
@@ -77,6 +87,44 @@ public final class AFPResourceAccessor {
      * @throws IOException if an I/O error occurs while creating the InputStream.
      */
     public InputStream createInputStream(URI uri) throws IOException {
-        return resourceResolver.getResource(getResourceURI(uri));
+        return resourceResolver.getResource(uriResolver.resolveURI(uri));
+    }
+
+    /**
+     * Returns the resolved URI, given the URI of a resource.
+     *
+     * @param uri the resource URI
+     * @return the resolved URI
+     */
+    public URI resolveURI(String uri) {
+        return uriResolver.resolveURI(uri);
+    }
+
+    private interface URIResolver {
+        URI resolveURI(URI uri);
+
+        URI resolveURI(String uri);
+    }
+
+    private final class NullBaseURIResolver implements URIResolver {
+
+        public URI resolveURI(URI uri) {
+            return uri;
+        }
+
+        public URI resolveURI(String uri) {
+            return URI.create("./" + uri.trim());
+        }
+    }
+
+    private final class BaseURIResolver implements URIResolver {
+
+        public URI resolveURI(URI uri) {
+            return baseURI.resolve(uri);
+        }
+
+        public URI resolveURI(String uri) {
+            return baseURI.resolve(uri.trim());
+        }
     }
 }

diff --git a/status.xml b/status.xml
index d7c43caa22a1a9268511656e3d1d3b9d46a46fda..bfb14e0dedfbc49933abbd1ec734703725ceed3c 100644 (file)
--- a/status.xml
+++ b/status.xml
@@ -62,6 +62,10 @@
       documents. Example: the fix of marks layering will be such a case when it's done.
     -->
     <release version="FOP Trunk" date="TBD">
+      <action context="Fonts" dev="MH" type="fix" fixes-bug="53685">
+        Cached AFP charactersets have more unique keys preventing the two characters with
+        but different binaries conflicting.
+      </action>
       <action context="Fonts" dev="MH" type="fix" fixes-bug="53657" due-to="Robert Meyer">
         AFP fonts default to the nominal character increment to font metrics when glyph info
         is missing from the characterset.