[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
JarFile fixes (Was: [Jikesrvm-regression] classpath CVS head regression
From: |
Mark Wielaard |
Subject: |
JarFile fixes (Was: [Jikesrvm-regression] classpath CVS head regression FAILED 4 tests; 0 mauve failures) |
Date: |
Thu, 11 Nov 2004 17:33:39 +0100 |
Hi,
On Tue, 2004-11-09 at 00:12, Mark Wielaard wrote:
> On Mon, 2004-11-08 at 23:46, Steven Augart wrote:
> > > development: Failed in SPECjbb2000
> > Another new problem, this time during class loading. I'm CC'ing this
> > to the classpath list, since it probably points to a problem in
> > Classpath CVS head.
>
> I was just hunting this one down.
> Attached is my first attempt.
>
> It makes sure to not check the verified Map when the jar file is
> explicitly opened with verify set to false. It also just compares the
> value in the Map to Boolean.TRUE or Boolean.FALSE.
>
> I am still thinking about synchronization issues.
> It looks like we need to be more careful about synchronization when
> manipulating/reading the verified Map since a JarFile can probably be
> accessed from multiple threads at once.
And here is the full fix:
2004-11-11 Mark Wielaard <address@hidden>
* java/util/jar/JarFile.java (verify): Make package private.
(signaturesRead): Likewise.
(verified): Likewise.
(entryCerts): Likewise.
(DEBUG): Likewise.
(debug): Likewise.
(entries): Construct new JarEnumeration with reference to this.
(JarEnumeration): Make static.
(JarEnumeration.jarfile): New field.
(JarEnumeration.nextElement): Use and synchronize on jarfile.
Compare verified value to Boolean.TRUE or Boolean.False only
when verify is true.
(getEntry): Make synchronized. Compare value of verified to
Boolean.TRUE.
(getInputStream): Construct EntryInputStream with reference to this.
(getManifest): Make synchronized.
(EntryInputStream): Make static.
(EntryInputStream.jarfile): New field.
(EntryInputStream.EntryInputStream): Check if manifest exists,
before getting attributes.
(eof): Synchronize on jarfile.
This fixes the issue when opening a JarFile with verify set to false,
makes JarFile thread-safe, and fixes a bug when the manifest file isn't
available at all.
Cheers,
Mark
Index: java/util/jar/JarFile.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/jar/JarFile.java,v
retrieving revision 1.13
diff -u -r1.13 JarFile.java
--- java/util/jar/JarFile.java 9 Nov 2004 05:51:47 -0000 1.13
+++ java/util/jar/JarFile.java 11 Nov 2004 16:24:41 -0000
@@ -122,23 +122,28 @@
private Manifest manifest;
/** Whether to verify the manifest and all entries. */
- private boolean verify;
+ boolean verify;
/** Whether the has already been loaded. */
private boolean manifestRead = false;
/** Whether the signature files have been loaded. */
- private boolean signaturesRead = false;
+ boolean signaturesRead = false;
- /** A map between entry names and booleans, signaling whether or
- not that entry has been verified. */
- private HashMap verified = new HashMap();
+ /**
+ * A map between entry names and booleans, signaling whether or
+ * not that entry has been verified.
+ * Only be accessed with lock on this JarFile*/
+ HashMap verified = new HashMap();
- /** A mapping from entry name to certificates, if any. */
- private HashMap entryCerts;
+ /**
+ * A mapping from entry name to certificates, if any.
+ * Only accessed with lock on this JarFile.
+ */
+ HashMap entryCerts;
- private static boolean DEBUG = false;
- private static void debug(Object msg)
+ static boolean DEBUG = false;
+ static void debug(Object msg)
{
System.err.print(JarFile.class.getName());
System.err.print(" >>> ");
@@ -303,21 +308,23 @@
*/
public Enumeration entries() throws IllegalStateException
{
- return new JarEnumeration(super.entries());
+ return new JarEnumeration(super.entries(), this);
}
/**
* Wraps a given Zip Entries Enumeration. For every zip entry a
* JarEntry is created and the corresponding Attributes are looked up.
*/
- private class JarEnumeration implements Enumeration
+ private static class JarEnumeration implements Enumeration
{
private final Enumeration entries;
+ private final JarFile jarfile;
- JarEnumeration(Enumeration e)
+ JarEnumeration(Enumeration e, JarFile f)
{
entries = e;
+ jarfile = f;
}
public boolean hasMoreElements()
@@ -332,7 +339,7 @@
Manifest manifest;
try
{
- manifest = getManifest();
+ manifest = jarfile.getManifest();
}
catch (IOException ioe)
{
@@ -344,32 +351,35 @@
jar.attr = manifest.getAttributes(jar.getName());
}
- if (!signaturesRead)
- try
- {
- readSignatures();
- }
- catch (IOException ioe)
- {
- if (DEBUG)
- {
- debug(ioe);
- ioe.printStackTrace();
- }
- signaturesRead = true; // fudge it.
- }
-
- // Include the certificates only if we have asserted that the
- // signatures are valid. This means the certificates will not be
- // available if the entry hasn't been read yet.
- if (entryCerts != null && verified.containsKey(zip.getName())
- && ((Boolean) verified.get(zip.getName())).booleanValue())
- {
- Set certs = (Set) entryCerts.get(jar.getName());
- if (certs != null)
- jar.certs = (Certificate[])
- certs.toArray(new Certificate[certs.size()]);
- }
+ synchronized(jarfile)
+ {
+ if (!jarfile.signaturesRead)
+ try
+ {
+ jarfile.readSignatures();
+ }
+ catch (IOException ioe)
+ {
+ if (JarFile.DEBUG)
+ {
+ JarFile.debug(ioe);
+ ioe.printStackTrace();
+ }
+ jarfile.signaturesRead = true; // fudge it.
+ }
+
+ // Include the certificates only if we have asserted that the
+ // signatures are valid. This means the certificates will not be
+ // available if the entry hasn't been read yet.
+ if (jarfile.entryCerts != null
+ && jarfile.verified.get(zip.getName()) == Boolean.TRUE)
+ {
+ Set certs = (Set) jarfile.entryCerts.get(jar.getName());
+ if (certs != null)
+ jar.certs = (Certificate[])
+ certs.toArray(new Certificate[certs.size()]);
+ }
+ }
return jar;
}
}
@@ -379,7 +389,7 @@
* It actually returns a JarEntry not a zipEntry
* @param name XXX
*/
- public ZipEntry getEntry(String name)
+ public synchronized ZipEntry getEntry(String name)
{
ZipEntry entry = super.getEntry(name);
if (entry != null)
@@ -400,32 +410,31 @@
jarEntry.attr = manifest.getAttributes(name);
}
- if (!signaturesRead)
- try
- {
- readSignatures();
- }
- catch (IOException ioe)
- {
- if (DEBUG)
- {
- debug(ioe);
- ioe.printStackTrace();
- }
- signaturesRead = true;
- }
- // See the comments in the JarEnumeration for why we do this
- // check.
- if (DEBUG)
- debug("entryCerts=" + entryCerts + " verified " + name
- + " ? " + verified.get(name));
- if (entryCerts != null && verified.containsKey(name)
- && ((Boolean) verified.get(name)).booleanValue())
- {
- Set certs = (Set) entryCerts.get(name);
- if (certs != null)
- jarEntry.certs = (Certificate[])
- certs.toArray(new Certificate[certs.size()]);
+ if (!signaturesRead)
+ try
+ {
+ readSignatures();
+ }
+ catch (IOException ioe)
+ {
+ if (DEBUG)
+ {
+ debug(ioe);
+ ioe.printStackTrace();
+ }
+ signaturesRead = true;
+ }
+ // See the comments in the JarEnumeration for why we do this
+ // check.
+ if (DEBUG)
+ debug("entryCerts=" + entryCerts + " verified " + name
+ + " ? " + verified.get(name));
+ if (entryCerts != null && verified.get(name) == Boolean.TRUE)
+ {
+ Set certs = (Set) entryCerts.get(name);
+ if (certs != null)
+ jarEntry.certs = (Certificate[])
+ certs.toArray(new Certificate[certs.size()]);
}
return jarEntry;
}
@@ -449,13 +458,13 @@
{
if (DEBUG)
debug("reading and verifying " + entry);
- return new EntryInputStream(entry, super.getInputStream(entry));
+ return new EntryInputStream(entry, super.getInputStream(entry), this);
}
else
{
if (DEBUG)
debug("reading already verified entry " + entry);
- if (!((Boolean) verified.get(entry.getName())).booleanValue())
+ if (verify && verified.get(entry.getName()) == Boolean.FALSE)
throw new ZipException("digest for " + entry + " is invalid");
return super.getInputStream(entry);
}
@@ -479,7 +488,7 @@
* Returns the manifest for this JarFile or null when the JarFile does not
* contain a manifest file.
*/
- public Manifest getManifest() throws IOException
+ public synchronized Manifest getManifest() throws IOException
{
if (!manifestRead)
manifest = readManifest();
@@ -487,6 +496,7 @@
return manifest;
}
+ // Only called with lock on this JarFile.
private void readSignatures() throws IOException
{
Map pkcs7Dsa = new HashMap();
@@ -872,8 +882,9 @@
/**
* A utility class that verifies jar entries as they are read.
*/
- private class EntryInputStream extends FilterInputStream
+ private static class EntryInputStream extends FilterInputStream
{
+ private final JarFile jarfile;
private final long length;
private long pos;
private final ZipEntry entry;
@@ -881,17 +892,25 @@
private final MessageDigest[] md;
private boolean checked;
- EntryInputStream(final ZipEntry entry, final InputStream in)
+ EntryInputStream(final ZipEntry entry,
+ final InputStream in,
+ final JarFile jar)
throws IOException
{
super(in);
this.entry = entry;
+ this.jarfile = jar;
length = entry.getSize();
pos = 0;
checked = false;
- Attributes attr = manifest.getAttributes(entry.getName());
+ Attributes attr;
+ Manifest manifest = jarfile.getManifest();
+ if (manifest != null)
+ attr = manifest.getAttributes(entry.getName());
+ else
+ attr = null;
if (DEBUG)
debug("verifying entry " + entry + " attr=" + attr);
if (attr == null)
@@ -1011,17 +1030,24 @@
+ " comp=" + new java.math.BigInteger(hash).toString(16));
if (!Arrays.equals(hash, hashes[i]))
{
- if (DEBUG)
- debug(entry + " could NOT be verified");
- verified.put(entry.getName(), Boolean.FALSE);
- return;
- // XXX ??? what do we do here?
- // throw new ZipException("message digest mismatch");
+ synchronized(jarfile)
+ {
+ if (DEBUG)
+ debug(entry + " could NOT be verified");
+ jarfile.verified.put(entry.getName(), Boolean.FALSE);
+ }
+ return;
+ // XXX ??? what do we do here?
+ // throw new ZipException("message digest mismatch");
}
}
- if (DEBUG)
- debug(entry + " has been VERIFIED");
- verified.put(entry.getName(), Boolean.TRUE);
+
+ synchronized(jarfile)
+ {
+ if (DEBUG)
+ debug(entry + " has been VERIFIED");
+ jarfile.verified.put(entry.getName(), Boolean.TRUE);
+ }
}
}
}
signature.asc
Description: This is a digitally signed message part