From ba60c0b3f9bbd00975c984244839b155e84b4c5d Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Mon, 1 Feb 2021 04:16:57 -0800 Subject: [PATCH] ijar: fix manifest sections handling Fixes #12730 Important changes: - removed line stripping from the original implementation - label/rule attributes are appended after main attributes, then the rest of the `MANIFEST.MF` content is appended - this way sections data is preserved. Closes #12771. PiperOrigin-RevId: 354909855 --- third_party/ijar/ijar.cc | 21 ++++-- third_party/ijar/test/BUILD | 45 +++++++++++++ .../ijar/test/GenJarWithManifestSections.java | 64 +++++++++++++++++++ third_party/ijar/test/IjarTests.java | 51 +++++++++++++++ 4 files changed, 175 insertions(+), 6 deletions(-) create mode 100644 third_party/ijar/test/GenJarWithManifestSections.java diff --git a/third_party/ijar/ijar.cc b/third_party/ijar/ijar.cc index 2119c06a0df449..67ac5028ca7fa6 100644 --- a/third_party/ijar/ijar.cc +++ b/third_party/ijar/ijar.cc @@ -302,7 +302,10 @@ u1 *JarCopierProcessor::AppendTargetLabelToManifest( const char *target_label, const char *injecting_rule_kind) { const char *line_start = (const char *)manifest_data; const char *data_end = (const char *)manifest_data + size; - while (line_start < data_end) { + + // Write main attributes part + while (line_start < data_end && line_start[0] != '\r' && + line_start[0] != '\n') { const char *line_end = strchr(line_start, '\n'); // Go past return char to point to next line, or to end of data buffer line_end = line_end != nullptr ? line_end + 1 : data_end; @@ -313,18 +316,24 @@ u1 *JarCopierProcessor::AppendTargetLabelToManifest( strncmp(line_start, INJECTING_RULE_KIND_KEY, INJECTING_RULE_KIND_KEY_LENGTH) != 0) { size_t len = line_end - line_start; - // Skip empty lines - if (len > 0 && line_start[0] != '\r' && line_start[0] != '\n') { - memcpy(buf, line_start, len); - buf += len; - } + memcpy(buf, line_start, len); + buf += len; } line_start = line_end; } + + // Append target label and, if given, rule kind buf = WriteManifestAttr(buf, TARGET_LABEL_KEY, target_label); if (injecting_rule_kind != nullptr) { buf = WriteManifestAttr(buf, INJECTING_RULE_KIND_KEY, injecting_rule_kind); } + + // Write the rest of the manifest file + size_t sections_len = data_end - line_start; + if (sections_len > 0) { + memcpy(buf, line_start, sections_len); + buf += sections_len; + } return buf; } diff --git a/third_party/ijar/test/BUILD b/third_party/ijar/test/BUILD index daf0f16a78bbcf..19ffb4abd44ff6 100644 --- a/third_party/ijar/test/BUILD +++ b/third_party/ijar/test/BUILD @@ -144,6 +144,24 @@ genrule( tools = ["//third_party/ijar"], ) +genrule( + name = "jar_with_manifest_sections_nostrip", + testonly = 1, + srcs = [":jar_with_manifest_sections"], + outs = ["jar-with-manifest-sections-nostrip.jar"], + cmd = "$(location //third_party/ijar) --target_label //foo:foo --nostrip_jar $< $@", + tools = ["//third_party/ijar"], +) + +genrule( + name = "jar_with_target_label_and_manifest_sections_nostrip", + testonly = 1, + srcs = [":jar_with_target_label_and_manifest_sections"], + outs = ["jar-with-target-label-and-manifest-sections-nostrip.jar"], + cmd = "$(location //third_party/ijar) --target_label //foo:foo --nostrip_jar $< $@", + tools = ["//third_party/ijar"], +) + genrule( name = "jar_without_manifest_nostrip_idempotence", srcs = ["jar-without-manifest-nostrip.jar"], @@ -292,6 +310,30 @@ genrule( tools = ["//third_party/ijar"], ) +java_binary( + name = "GenJarWithManifestSections", + testonly = 1, + srcs = ["GenJarWithManifestSections.java"], + main_class = "GenJarWithManifestSections", + deps = ["//third_party:guava"], +) + +genrule( + name = "jar_with_manifest_sections", + testonly = 1, + outs = ["jar-with-manifest-sections.jar"], + cmd = "$(location :GenJarWithManifestSections) $@", + tools = [":GenJarWithManifestSections"], +) + +genrule( + name = "jar_with_target_label_and_manifest_sections", + testonly = 1, + outs = ["jar-with-target-label-and-manifest-sections.jar"], + cmd = "$(location :GenJarWithManifestSections) $@ //not:this", + tools = [":GenJarWithManifestSections"], +) + java_test( name = "IjarTests", size = "small", @@ -305,6 +347,7 @@ java_test( "UseRestrictedAnnotation.java", "jar-with-manifest.jar", "jar-with-manifest-and-target-label.jar", + "jar-with-manifest-sections.jar", "jar-without-manifest.jar", "package-info.java", ":empty_with_target_label", @@ -314,6 +357,8 @@ java_test( ":interface_ijar_testlib_with_target_label", ":jar_with_manifest_and_target_label_nostrip", ":jar_with_manifest_nostrip", + ":jar_with_manifest_sections_nostrip", + ":jar_with_target_label_and_manifest_sections_nostrip", ":jar_without_manifest_nostrip", ":jar_without_manifest_nostrip_idempotence", ":kotlin_module-interface.jar", diff --git a/third_party/ijar/test/GenJarWithManifestSections.java b/third_party/ijar/test/GenJarWithManifestSections.java new file mode 100644 index 00000000000000..44d3d96bb56ddb --- /dev/null +++ b/third_party/ijar/test/GenJarWithManifestSections.java @@ -0,0 +1,64 @@ +// Copyright 2021 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import com.google.common.io.ByteStreams; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.Map; +import java.util.jar.Attributes; +import java.util.jar.JarOutputStream; +import java.util.jar.Manifest; +import java.util.zip.ZipEntry; + +/** Writes a jar file with manifest and optionally a Target-Label attribute. */ +public final class GenJarWithManifestSections { + + public static void main(String[] args) throws IOException { + try (JarOutputStream jos = new JarOutputStream(Files.newOutputStream(Paths.get(args[0])))) { + addEntry(jos, "META-INF/MANIFEST.MF"); + Manifest manifest = new Manifest(); + Attributes mainAttributes = manifest.getMainAttributes(); + mainAttributes.put(Attributes.Name.MANIFEST_VERSION, "1.0"); + + if (args.length > 1) { + String targetLabel = args[1]; + mainAttributes.put(new Attributes.Name("Target-Label"), targetLabel); + } + + Map entries = manifest.getEntries(); + + Attributes foo = new Attributes(); + foo.put(new Attributes.Name("Foo"), "bar"); + entries.put("foo", foo); + + Attributes baz = new Attributes(); + baz.put(new Attributes.Name("Another"), "bar"); + entries.put("baz", baz); + + manifest.write(jos); + + addEntry(jos, "java/lang/String.class"); + ByteStreams.copy(String.class.getResourceAsStream("/java/lang/String.class"), jos); + } + } + + private static void addEntry(JarOutputStream jos, String name) throws IOException { + ZipEntry ze = new ZipEntry(name); + ze.setTime(0); + jos.putNextEntry(ze); + } + + private GenJarWithManifestSections() {} +} diff --git a/third_party/ijar/test/IjarTests.java b/third_party/ijar/test/IjarTests.java index 7563f599ca39c0..862ad6e32da219 100644 --- a/third_party/ijar/test/IjarTests.java +++ b/third_party/ijar/test/IjarTests.java @@ -399,6 +399,57 @@ public void testNoStripJarWithoutManifest() throws Exception { } } + @Test + public void testPreserveManifestSections() throws Exception { + try (JarFile original = + new JarFile( + "third_party/ijar/test/jar-with-target-label-and-manifest-sections-nostrip.jar"); + JarFile stripped = + new JarFile("third_party/ijar/test/jar-with-manifest-sections-nostrip.jar")) { + ImmutableList strippedEntries = + stripped.stream().map(JarEntry::getName).collect(toImmutableList()); + + assertThat(strippedEntries.get(0)).isEqualTo("META-INF/"); + assertThat(strippedEntries.get(1)).isEqualTo("META-INF/MANIFEST.MF"); + Manifest manifest = stripped.getManifest(); + Attributes attributes = manifest.getMainAttributes(); + assertThat(attributes.getValue("Target-Label")).isEqualTo("//foo:foo"); + assertNonManifestFilesBitIdentical(original, stripped); + + Attributes sectionAttributes1 = manifest.getAttributes("foo"); + assertThat(sectionAttributes1.getValue("Foo")).isEqualTo("bar"); + + Attributes sectionAttributes2 = manifest.getAttributes("baz"); + assertThat(sectionAttributes2.getValue("Another")).isEqualTo("bar"); + } + } + + @Test + public void testPreserveManifestSectionsAndUpdateExistingTargetLabel() throws Exception { + try (JarFile original = + new JarFile( + "third_party/ijar/test/jar-with-target-label-and-manifest-sections-nostrip.jar"); + JarFile stripped = + new JarFile( + "third_party/ijar/test/jar-with-target-label-and-manifest-sections-nostrip.jar")) { + ImmutableList strippedEntries = + stripped.stream().map(JarEntry::getName).collect(toImmutableList()); + + assertThat(strippedEntries.get(0)).isEqualTo("META-INF/"); + assertThat(strippedEntries.get(1)).isEqualTo("META-INF/MANIFEST.MF"); + Manifest manifest = stripped.getManifest(); + Attributes attributes = manifest.getMainAttributes(); + assertThat(attributes.getValue("Target-Label")).isEqualTo("//foo:foo"); + assertNonManifestFilesBitIdentical(original, stripped); + + Attributes sectionAttributes1 = manifest.getAttributes("foo"); + assertThat(sectionAttributes1.getValue("Foo")).isEqualTo("bar"); + + Attributes sectionAttributes2 = manifest.getAttributes("baz"); + assertThat(sectionAttributes2.getValue("Another")).isEqualTo("bar"); + } + } + // Tests idempotence of --nostrip @Test public void testNoStripIdempotence() throws Exception {