AAPT2: Fix file::mkdirs when given absolute paths

file::mkdirs would try to extract the first part of the
path '/path' which would be the empty string ''. Mkdir would
fail creating the empty string directory.

Change-Id: Ice8ba92135f145f52f3663499a2c49eebe797328

6 files changed, 105 insertions, 32 deletions

diff --git a/tools/aapt2/Android.mk b/tools/aapt2/Android.mk
index 57a769221a8f..d311b3d41a70 100644
--- a/tools/aapt2/Android.mk
+++ b/tools/aapt2/Android.mk
@@ -90,6 +90,7 @@ testSources := \
 	proto/TableProtoSerializer_test.cpp \
 	split/TableSplitter_test.cpp \
 	util/BigBuffer_test.cpp \
+	util/Files_test.cpp \
 	util/Maybe_test.cpp \
 	util/StringPiece_test.cpp \
 	util/Util_test.cpp \
diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h
index 4d1db5b0c9ad..03ca42b286d6 100644
--- a/tools/aapt2/Resource.h
+++ b/tools/aapt2/Resource.h
@@ -77,7 +77,7 @@ struct ResourceName {
     ResourceType type;
     std::u16string entry;
 
-    ResourceName() = default;
+    ResourceName() : type(ResourceType::kRaw) {}
     ResourceName(const StringPiece16& p, ResourceType t, const StringPiece16& e);
 
     bool isValid() const;
diff --git a/tools/aapt2/link/Link.cpp b/tools/aapt2/link/Link.cpp
index 5003d9658525..418a802983c3 100644
--- a/tools/aapt2/link/Link.cpp
+++ b/tools/aapt2/link/Link.cpp
@@ -729,12 +729,18 @@ public:
 
         std::string outPath = mOptions.generateJavaClassPath.value();
         file::appendPath(&outPath, file::packageToPath(util::utf16ToUtf8(outPackage)));
-        file::mkdirs(outPath);
+        if (!file::mkdirs(outPath)) {
+            mContext->getDiagnostics()->error(
+                    DiagMessage() << "failed to create directory '" << outPath << "'");
+            return false;
+        }
+
         file::appendPath(&outPath, "R.java");
 
         std::ofstream fout(outPath, std::ofstream::binary);
         if (!fout) {
-            mContext->getDiagnostics()->error(DiagMessage() << strerror(errno));
+            mContext->getDiagnostics()->error(
+                    DiagMessage() << "failed writing to '" << outPath << "': " << strerror(errno));
             return false;
         }
 
@@ -743,6 +749,11 @@ public:
             mContext->getDiagnostics()->error(DiagMessage(outPath) << generator.getError());
             return false;
         }
+
+        if (!fout) {
+            mContext->getDiagnostics()->error(
+                    DiagMessage() << "failed writing to '" << outPath << "': " << strerror(errno));
+        }
         return true;
     }
 
@@ -754,12 +765,18 @@ public:
         std::string outPath = mOptions.generateJavaClassPath.value();
         file::appendPath(&outPath,
                          file::packageToPath(util::utf16ToUtf8(mContext->getCompilationPackage())));
-        file::mkdirs(outPath);
+        if (!file::mkdirs(outPath)) {
+            mContext->getDiagnostics()->error(
+                    DiagMessage() << "failed to create directory '" << outPath << "'");
+            return false;
+        }
+
         file::appendPath(&outPath, "Manifest.java");
 
         std::ofstream fout(outPath, std::ofstream::binary);
         if (!fout) {
-            mContext->getDiagnostics()->error(DiagMessage() << strerror(errno));
+            mContext->getDiagnostics()->error(
+                    DiagMessage() << "failed writing to '" << outPath << "': " << strerror(errno));
             return false;
         }
 
@@ -770,7 +787,8 @@ public:
         }
 
         if (!fout) {
-            mContext->getDiagnostics()->error(DiagMessage() << strerror(errno));
+            mContext->getDiagnostics()->error(
+                    DiagMessage() << "failed writing to '" << outPath << "': " << strerror(errno));
             return false;
         }
         return true;
@@ -781,15 +799,18 @@ public:
             return true;
         }
 
-        std::ofstream fout(mOptions.generateProguardRulesPath.value(), std::ofstream::binary);
+        const std::string& outPath = mOptions.generateProguardRulesPath.value();
+        std::ofstream fout(outPath, std::ofstream::binary);
         if (!fout) {
-            mContext->getDiagnostics()->error(DiagMessage() << strerror(errno));
+            mContext->getDiagnostics()->error(
+                    DiagMessage() << "failed to open '" << outPath << "': " << strerror(errno));
             return false;
         }
 
         proguard::writeKeepSet(&fout, keepSet);
         if (!fout) {
-            mContext->getDiagnostics()->error(DiagMessage() << strerror(errno));
+            mContext->getDiagnostics()->error(
+                    DiagMessage() << "failed writing to '" << outPath << "': " << strerror(errno));
             return false;
         }
         return true;
diff --git a/tools/aapt2/util/Files.cpp b/tools/aapt2/util/Files.cpp
index 04e8199d85b4..6428e9867334 100644
--- a/tools/aapt2/util/Files.cpp
+++ b/tools/aapt2/util/Files.cpp
@@ -96,7 +96,7 @@ bool mkdirs(const StringPiece& path) {
     const char* start = path.begin();
     const char* end = path.end();
     for (const char* current = start; current != end; ++current) {
-        if (*current == sDirSep) {
+        if (*current == sDirSep && current != start) {
             StringPiece parentPath(start, current - start);
             int result = mkdirImpl(parentPath);
             if (result < 0 && errno != EEXIST) {
@@ -139,6 +139,20 @@ StringPiece getExtension(const StringPiece& path) {
     return {};
 }
 
+void appendPath(std::string* base, StringPiece part) {
+    assert(base);
+    const bool baseHasTrailingSep = (!base->empty() && *(base->end() - 1) == sDirSep);
+    const bool partHasLeadingSep = (!part.empty() && *(part.begin()) == sDirSep);
+    if (baseHasTrailingSep && partHasLeadingSep) {
+        // Remove the part's leading sep
+        part = part.substr(1, part.size() - 1);
+    } else if (!baseHasTrailingSep && !partHasLeadingSep) {
+        // None of the pieces has a separator.
+        *base += sDirSep;
+    }
+    base->append(part.data(), part.size());
+}
+
 std::string packageToPath(const StringPiece& package) {
     std::string outPath;
     for (StringPiece part : util::tokenize<char>(package, '.')) {
diff --git a/tools/aapt2/util/Files.h b/tools/aapt2/util/Files.h
index c58ba5d6d1e3..c2e611543b1e 100644
--- a/tools/aapt2/util/Files.h
+++ b/tools/aapt2/util/Files.h
@@ -61,14 +61,7 @@ std::vector<std::string> listFiles(const StringPiece& root);
 /*
  * Appends a path to `base`, separated by the directory separator.
  */
-void appendPath(std::string* base, const StringPiece& part);
-
-/*
- * Appends a series of paths to `base`, separated by the
- * system directory separator.
- */
-template <typename... Ts >
-void appendPath(std::string* base, const StringPiece& part, const Ts&... parts);
+void appendPath(std::string* base, StringPiece part);
 
 /*
  * Makes all the directories in `path`. The last element in the path
@@ -139,20 +132,6 @@ private:
     std::vector<std::string> mPatternTokens;
 };
 
-inline void appendPath(std::string* base, const StringPiece& part) {
-    assert(base);
-    *base += sDirSep;
-    base->append(part.data(), part.size());
-}
-
-template <typename... Ts >
-void appendPath(std::string* base, const StringPiece& part, const Ts&... parts) {
-    assert(base);
-    *base += sDirSep;
-    base->append(part.data(), part.size());
-    appendPath(base, parts...);
-}
-
 } // namespace file
 } // namespace aapt
 
diff --git a/tools/aapt2/util/Files_test.cpp b/tools/aapt2/util/Files_test.cpp
new file mode 100644
index 000000000000..efb04593ff82
--- /dev/null
+++ b/tools/aapt2/util/Files_test.cpp
@@ -0,0 +1,58 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * 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.
+ */
+
+#include "test/Test.h"
+#include "util/Files.h"
+
+#include <sstream>
+
+namespace aapt {
+namespace file {
+
+class FilesTest : public ::testing::Test {
+public:
+    void SetUp() override {
+        std::stringstream builder;
+        builder << "hello" << sDirSep << "there";
+        mExpectedPath = builder.str();
+    }
+
+protected:
+    std::string mExpectedPath;
+};
+
+TEST_F(FilesTest, appendPath) {
+    std::string base = "hello";
+    appendPath(&base, "there");
+    EXPECT_EQ(mExpectedPath, base);
+}
+
+TEST_F(FilesTest, appendPathWithLeadingOrTrailingSeparators) {
+    std::string base = "hello/";
+    appendPath(&base, "there");
+    EXPECT_EQ(mExpectedPath, base);
+
+    base = "hello";
+    appendPath(&base, "/there");
+    EXPECT_EQ(mExpectedPath, base);
+
+    base = "hello/";
+    appendPath(&base, "/there");
+    EXPECT_EQ(mExpectedPath, base);
+}
+
+} // namespace files
+} // namespace aapt