On Tue, 17 Jan 2023 16:11:42 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> This duplicated check annoyed me also, but the existing checks have >> different behavior: >> >> - JarVerifier.beginEntry normalizes the path to uppercase, them checks that >> it starts with "META-INF/" or "/META-INF/" >> - JarSigner.sign0 does not normalize to uppercase , then checks that the >> path starts with "META-INF/" >> >> Introducing a common method would need change behaviour of one of these >> methods. This change of behaviour would not be relevant to the bug being >> fixed in this PR. >> >> Since I'm cautious of changing behaviour, I decided to keep the two methods. > > While `JarSigner` has not normalize to uppercase, the check is the same. As > for `/META-INF/`, it must be very broken now since > `JarFile::maybeInstantiateVerifier` is using > `JUZFA.getManifestName(this,true)` to read the manifest and since `ZipFile` > cannot see the signature-related files starting with `/META-INF/` this method > will always return null. I agree this deserves a cleanup, and since we're already deep into it it can make sense to fix this in this PR. I have introduced SignatureFileVerifier.isInMetaInf as a common method to replace duplicated logic in JarVerifier and JarSigner. I also noticed that SignatureFileVerifier.isSigningRelated performs the same check, so I updated that method to also call the isInMetaInf. This introduces two subtle behavioural changes: - JarSigner is relaxed to ignore case when checking isInMetaInf, meaning "meta-inf/" files will now be moved to the beginning of the signed JAR - JarVerifier is tightened to reject /META-INF/ as not in META-INF/. This is believed to have little practical impact, since ZipFile.Source.isMetaName already rejects such paths. ------------- PR: https://git.openjdk.org/jdk/pull/11976
