From 614d1a6d815e3c03becd32ff44d4ff63d2235650 Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Mon, 9 Jan 2017 22:55:00 +0000 Subject: [PATCH] TarWriter: Fix a bug in Ustar header. If we split a filename into `Name` and `Prefix`, `Prefix` is at most 145 bytes. We had a bug that didn't split a path correctly. This bug was pointed out by Rafael in the post commit review. This patch adds a unit test for TarWriter to verify the fix. llvm-svn: 291494 --- lib/Support/TarWriter.cpp | 2 +- unittests/Support/CMakeLists.txt | 3 +- unittests/Support/TarWriterTest.cpp | 88 +++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 unittests/Support/TarWriterTest.cpp diff --git a/lib/Support/TarWriter.cpp b/lib/Support/TarWriter.cpp index 58abffcca55..f06abf46cce 100644 --- a/lib/Support/TarWriter.cpp +++ b/lib/Support/TarWriter.cpp @@ -122,7 +122,7 @@ static void writePaxHeader(raw_fd_ostream &OS, StringRef Path) { static std::pair splitPath(StringRef Path) { if (Path.size() <= sizeof(UstarHeader::Name)) return {"", Path}; - size_t Sep = Path.rfind('/', sizeof(UstarHeader::Name) + 1); + size_t Sep = Path.rfind('/', sizeof(UstarHeader::Prefix) + 1); if (Sep == StringRef::npos) return {"", Path}; return {Path.substr(0, Sep), Path.substr(Sep + 1)}; diff --git a/unittests/Support/CMakeLists.txt b/unittests/Support/CMakeLists.txt index 2ffedab82ac..6068de5514c 100644 --- a/unittests/Support/CMakeLists.txt +++ b/unittests/Support/CMakeLists.txt @@ -43,10 +43,11 @@ add_llvm_unittest(SupportTests SpecialCaseListTest.cpp StringPool.cpp SwapByteOrderTest.cpp + TarWriterTest.cpp TargetParserTest.cpp - Threading.cpp ThreadLocalTest.cpp ThreadPool.cpp + Threading.cpp TimerTest.cpp TypeNameTest.cpp TrailingObjectsTest.cpp diff --git a/unittests/Support/TarWriterTest.cpp b/unittests/Support/TarWriterTest.cpp new file mode 100644 index 00000000000..dfcf88f16ae --- /dev/null +++ b/unittests/Support/TarWriterTest.cpp @@ -0,0 +1,88 @@ +//===- llvm/unittest/Support/TarWriterTest.cpp ----------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/TarWriter.h" +#include "gtest/gtest.h" + +using namespace llvm; +namespace { + +struct UstarHeader { + char Name[100]; + char Mode[8]; + char Uid[8]; + char Gid[8]; + char Size[12]; + char Mtime[12]; + char Checksum[8]; + char TypeFlag; + char Linkname[100]; + char Magic[6]; + char Version[2]; + char Uname[32]; + char Gname[32]; + char DevMajor[8]; + char DevMinor[8]; + char Prefix[155]; + char Pad[12]; +}; + +class TarWriterTest : public ::testing::Test {}; + +static UstarHeader create(StringRef Base, StringRef Filename) { + // Create a temporary file. + SmallString<128> Path; + std::error_code EC = + sys::fs::createTemporaryFile("TarWriterTest", "tar", Path); + EXPECT_FALSE((bool)EC); + + // Create a tar file. + Expected> TarOrErr = TarWriter::create(Path, Base); + EXPECT_TRUE((bool)TarOrErr); + std::unique_ptr Tar = std::move(*TarOrErr); + Tar->append(Filename, "contents"); + Tar.release(); + + // Read the tar file. + ErrorOr> MBOrErr = MemoryBuffer::getFile(Path); + EXPECT_TRUE((bool)MBOrErr); + std::unique_ptr MB = std::move(*MBOrErr); + sys::fs::remove(Path); + return *reinterpret_cast(MB->getBufferStart()); +} + +TEST_F(TarWriterTest, Basics) { + UstarHeader Hdr = create("base", "file"); + ASSERT_EQ("ustar", StringRef(Hdr.Magic)); + ASSERT_EQ("00", StringRef(Hdr.Version, 2)); + ASSERT_EQ("base/file", StringRef(Hdr.Name)); + ASSERT_EQ("00000000010", StringRef(Hdr.Size)); +} + +TEST_F(TarWriterTest, LongFilename) { + UstarHeader Hdr1 = create( + "012345678", std::string(99, 'x') + "/" + std::string(44, 'x') + "/foo"); + ASSERT_EQ("foo", StringRef(Hdr1.Name)); + ASSERT_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"); + ASSERT_EQ("foo", StringRef(Hdr2.Name)); + ASSERT_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"); + ASSERT_EQ(std::string(46, 'x') + "/foo", StringRef(Hdr3.Name)); + ASSERT_EQ("012345678/" + std::string(99, 'x'), StringRef(Hdr3.Prefix)); +} +}