ijar: fix manifest sections handling

third_party/ijar/ijar.cc

@@ -298,7 +298,9 @@ 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;
@@ -309,18 +311,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;
 }
 

third_party/ijar/test/BUILD

@@ -144,6 +144,24 @@ genrule(
     tools = ["//third_party/ijar"],
 )
 
+genrule(
+    name = "jar_with_manifest_sections_nostrip",
+    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"],
+    testonly = 1,
+)
+
+genrule(
+    name = "jar_with_target_label_and_manifest_sections_nostrip",
+    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"],
+    testonly = 1,
+)
+
 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",
+    main_class = "GenJarWithManifestSections",
+    srcs = ["GenJarWithManifestSections.java"],
+    deps = ["//third_party:guava"],
+    testonly = 1,
+)
+
+genrule(
+    name = "jar_with_manifest_sections",
+    outs = ["jar-with-manifest-sections.jar"],
+    cmd = "$(location :GenJarWithManifestSections) $@",
+    testonly = 1,
+    tools = [":GenJarWithManifestSections"],
+)
+
+genrule(
+    name = "jar_with_target_label_and_manifest_sections",
+    outs = ["jar-with-target-label-and-manifest-sections.jar"],
+    cmd = "$(location :GenJarWithManifestSections) $@ //not:this",
+    testonly = 1,
+    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",

third_party/ijar/test/GenJarWithManifestSections.java

@@ -0,0 +1,61 @@
+// 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;
+
+public 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<String, Attributes> 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);
+  }
+}

third_party/ijar/test/IjarTests.java

@@ -399,6 +399,55 @@ 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<String> 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<String> 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 {