Fix off-by-one error in TarWriter.

The tar format originally supported up to 99 byte filename. The two
extensions are proposed later: Ustar or PAX.

In the UStar extension, a pathanme is split at a '/' and its "prefix"
and "suffix" are stored in different locations in the tar header. Since
"prefix" can be up to 155 byte, it can represent up to 254 byte
filename (but exact limit depends on the location of '/' character in
a pathname.)

Our TarWriter first attempt to use UStar extension and then fallback to
PAX extension.

But there's a bug in UStar header creation. "Suffix" part must be a NUL-
terminated string, but we didn't handle it correctly. As a result, if
your filename just 100 characters long, the last character was droppped.

This patch fixes the issue.

Differential Revision: https://reviews.llvm.org/D38149

llvm-svn: 314349

1 files changed, 54 insertions, 19 deletions

diff --git a/llvm/unittests/Support/TarWriterTest.cpp b/llvm/unittests/Support/TarWriterTest.cpp
index 927c8ed9be1..6007e73ffaf 100644
--- a/llvm/unittests/Support/TarWriterTest.cpp
+++ b/llvm/unittests/Support/TarWriterTest.cpp
@@ -11,6 +11,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "gtest/gtest.h"
+#include <vector>
 
 using namespace llvm;
 namespace {
@@ -37,7 +38,7 @@ struct UstarHeader {
 
 class TarWriterTest : public ::testing::Test {};
 
-static UstarHeader create(StringRef Base, StringRef Filename) {
+static std::vector<uint8_t> createTar(StringRef Base, StringRef Filename) {
   // Create a temporary file.
   SmallString<128> Path;
   std::error_code EC =
@@ -55,12 +56,25 @@ static UstarHeader create(StringRef Base, StringRef Filename) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr = MemoryBuffer::getFile(Path);
   EXPECT_TRUE((bool)MBOrErr);
   std::unique_ptr<MemoryBuffer> MB = std::move(*MBOrErr);
+  std::vector<uint8_t> Buf((const uint8_t *)MB->getBufferStart(),
+                           (const uint8_t *)MB->getBufferEnd());
+
+  // Windows does not allow us to remove a mmap'ed files, so
+  // unmap first and then remove the temporary file.
+  MB = nullptr;
   sys::fs::remove(Path);
-  return *reinterpret_cast<const UstarHeader *>(MB->getBufferStart());
+
+  return Buf;
+}
+
+static UstarHeader createUstar(StringRef Base, StringRef Filename) {
+  std::vector<uint8_t> Buf = createTar(Base, Filename);
+  EXPECT_TRUE(Buf.size() >= sizeof(UstarHeader));
+  return *reinterpret_cast<const UstarHeader *>(Buf.data());
 }
 
 TEST_F(TarWriterTest, Basics) {
-  UstarHeader Hdr = create("base", "file");
+  UstarHeader Hdr = createUstar("base", "file");
   EXPECT_EQ("ustar", StringRef(Hdr.Magic));
   EXPECT_EQ("00", StringRef(Hdr.Version, 2));
   EXPECT_EQ("base/file", StringRef(Hdr.Name));
@@ -68,21 +82,42 @@ TEST_F(TarWriterTest, Basics) {
 }
 
 TEST_F(TarWriterTest, LongFilename) {
-  UstarHeader Hdr1 = create(
-      "012345678", std::string(99, 'x') + "/" + std::string(44, 'x') + "/foo");
-  EXPECT_EQ("foo", StringRef(Hdr1.Name));
-  EXPECT_EQ("012345678/" + std::string(99, 'x') + "/" + std::string(44, 'x'),
-            StringRef(Hdr1.Prefix));
-
-  UstarHeader Hdr2 = create(
-      "012345678", std::string(99, 'x') + "/" + std::string(45, 'x') + "/foo");
-  EXPECT_EQ("foo", StringRef(Hdr2.Name));
-  EXPECT_EQ("012345678/" + std::string(99, 'x') + "/" + std::string(45, 'x'),
-            StringRef(Hdr2.Prefix));
-
-  UstarHeader Hdr3 = create(
-      "012345678", std::string(99, 'x') + "/" + std::string(46, 'x') + "/foo");
-  EXPECT_EQ(std::string(46, 'x') + "/foo", StringRef(Hdr3.Name));
-  EXPECT_EQ("012345678/" + std::string(99, 'x'), StringRef(Hdr3.Prefix));
+  std::string x154(154, 'x');
+  std::string x155(155, 'x');
+  std::string y99(99, 'y');
+  std::string y100(100, 'y');
+
+  UstarHeader Hdr1 = createUstar("", x154 + "/" + y99);
+  EXPECT_EQ("/" + x154, StringRef(Hdr1.Prefix));
+  EXPECT_EQ(y99, StringRef(Hdr1.Name));
+
+  UstarHeader Hdr2 = createUstar("", x155 + "/" + y99);
+  EXPECT_EQ("", StringRef(Hdr2.Prefix));
+  EXPECT_EQ("", StringRef(Hdr2.Name));
+
+  UstarHeader Hdr3 = createUstar("", x154 + "/" + y100);
+  EXPECT_EQ("", StringRef(Hdr3.Prefix));
+  EXPECT_EQ("", StringRef(Hdr3.Name));
+
+  UstarHeader Hdr4 = createUstar("", x155 + "/" + y100);
+  EXPECT_EQ("", StringRef(Hdr4.Prefix));
+  EXPECT_EQ("", StringRef(Hdr4.Name));
+
+  std::string yz = "yyyyyyyyyyyyyyyyyyyy/zzzzzzzzzzzzzzzzzzzz";
+  UstarHeader Hdr5 = createUstar("", x154 + "/" + yz);
+  EXPECT_EQ("/" + x154, StringRef(Hdr5.Prefix));
+  EXPECT_EQ(yz, StringRef(Hdr5.Name));
+}
+
+TEST_F(TarWriterTest, Pax) {
+  std::vector<uint8_t> Buf = createTar("", std::string(200, 'x'));
+  EXPECT_TRUE(Buf.size() >= 1024);
+
+  auto *Hdr = reinterpret_cast<const UstarHeader *>(Buf.data());
+  EXPECT_EQ("", StringRef(Hdr->Prefix));
+  EXPECT_EQ("", StringRef(Hdr->Name));
+
+  StringRef Pax = StringRef((char *)(Buf.data() + 512), 512);
+  EXPECT_TRUE(Pax.startswith("211 path=/" + std::string(200, 'x')));
 }
 }