On Nov 25, 2006, at 3:59 PM, Raif S. Naffah wrote:
hello Casey,
On Tuesday 21 November 2006 08:35, Casey Marshall wrote:
...
This patch fixes an issue with JarEntry objects that are created
before the entry is verified: we used to bind the certificates to a
JarEntry at creation time, so if the entry has not been verified yet,
the certificates field will be null. This is contrary to the JDK
behavior.
This changes JarEntry to hold a pointer of its containing JarFile,
and getCertificates now just accesses the "entryCerts" field of
JarFile directly. Thus, once an entry is verified, its certificates
may be looked up with a previously-initialized entry...
i've added a new method to the Mauve test in
gnu/testlet/java/util/jar/JarFile/TestOfManifest.java to highlight
this
issue.
with both RI 1.4 (1.4.2_13) and 1.5 (1.5.0_09) the behavior is the
same: the
counts of certificates is 0, 1, and 1 respectively. before your
patch our
implementation outputs 0, 0, and 1, and after the patch: 1, 1, and
1 which is
different in both cases.
Yeah, JarEntry needs to check if the entry has been verified first
before looking in the certificates map. Attached is an updated patch;
with it the TestOfManifest test passes.
### Eclipse Workspace Patch 1.0
#P classpath
Index: java/util/jar/JarEntry.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/jar/JarEntry.java,v
retrieving revision 1.6
diff -u -r1.6 JarEntry.java
--- java/util/jar/JarEntry.java 2 Jul 2005 20:32:44 -0000 1.6
+++ java/util/jar/JarEntry.java 26 Nov 2006 08:39:02 -0000
@@ -39,6 +39,7 @@
import java.io.IOException;
import java.security.cert.Certificate;
+import java.util.Set;
import java.util.zip.ZipEntry;
/**
@@ -60,7 +61,7 @@
// (Package local) fields
Attributes attr;
- Certificate certs[];
+ JarFile jarfile;
// Constructors
@@ -79,7 +80,7 @@
{
super(name);
attr = null;
- certs = null;
+ jarfile = null;
}
/**
@@ -93,7 +94,7 @@
{
super(entry);
attr = null;
- certs = null;
+ jarfile = null;
}
/**
@@ -112,7 +113,7 @@
catch (IOException _)
{
}
- certs = entry.getCertificates();
+ jarfile = entry.jarfile;
}
// Methods
@@ -153,13 +154,17 @@
*/
public Certificate[] getCertificates()
{
- if (certs != null)
+ if (jarfile != null)
{
- return (Certificate[])certs.clone();
- }
- else
- {
- return null;
+ // XXX synchronization???
+ if (jarfile.entryCerts != null)
+ {
+ Set certs = (Set) jarfile.entryCerts.get(getName());
+ if (certs != null
+ && jarfile.verified.get(getName()) == Boolean.TRUE)
+ return (Certificate[]) certs.toArray(new
Certificate[certs.size()]);
+ }
}
+ return null;
}
}
Index: java/util/jar/JarFile.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/jar/JarFile.java,v
retrieving revision 1.23
diff -u -r1.23 JarFile.java
--- java/util/jar/JarFile.java 20 Nov 2006 09:28:49 -0000 1.23
+++ java/util/jar/JarFile.java 26 Nov 2006 08:39:02 -0000
@@ -382,19 +382,8 @@
}
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()]);
- }
}
+ jar.jarfile = jarfile;
return jar;
}
}
@@ -439,18 +428,7 @@
}
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()]);
- }
+ jarEntry.jarfile = this;
return jarEntry;
}
return null;
