Merge changes I0a00b3d4,I19cb0694

* changes:
  adb: let `adb push` follow symlinks.
  base: add parameter that controls O_NOFOLLOW in file functions.

adb/file_sync_client.cpp

@@ -606,7 +606,7 @@ static bool sync_send(SyncConnection& sc, const char* lpath, const char* rpath,
     }
     if (st.st_size < SYNC_DATA_MAX) {
         std::string data;
-        if (!android::base::ReadFileToString(lpath, &data)) {
+        if (!android::base::ReadFileToString(lpath, &data, true)) {
             sc.Error("failed to read all of '%s': %s", lpath, strerror(errno));
             return false;
         }

adb/test_device.py

@@ -787,6 +787,36 @@ class FileOperationsTest(DeviceTest):
             if host_dir is not None:
                 shutil.rmtree(host_dir)
 
+    @unittest.skipIf(sys.platform == "win32", "symlinks require elevated privileges on windows")
+    def test_push_symlink(self):
+        """Push a symlink.
+
+        Bug: http://b/31491920
+        """
+        try:
+            host_dir = tempfile.mkdtemp()
+
+            # Make sure the temp directory isn't setuid, or else adb will
+            # complain.
+            os.chmod(host_dir, 0o700)
+
+            with open(os.path.join(host_dir, 'foo'), 'w') as f:
+                f.write('foo')
+
+            symlink_path = os.path.join(host_dir, 'symlink')
+            os.symlink('foo', symlink_path)
+
+            self.device.shell(['rm', '-rf', self.DEVICE_TEMP_DIR])
+            self.device.shell(['mkdir', self.DEVICE_TEMP_DIR])
+            self.device.push(symlink_path, self.DEVICE_TEMP_DIR)
+            rc, out, _ = self.device.shell_nocheck(
+                ['cat', posixpath.join(self.DEVICE_TEMP_DIR, 'symlink')])
+            self.assertEqual(0, rc)
+            self.assertEqual(out.strip(), 'foo')
+        finally:
+            if host_dir is not None:
+                shutil.rmtree(host_dir)
+
     def test_multiple_push(self):
         """Push multiple files to the device in one adb push command.
 

base/file.cpp

@@ -55,10 +55,11 @@ bool ReadFdToString(int fd, std::string* content) {
   return (n == 0) ? true : false;
 }
 
-bool ReadFileToString(const std::string& path, std::string* content) {
+bool ReadFileToString(const std::string& path, std::string* content, bool follow_symlinks) {
   content->clear();
 
-  int fd = TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY | O_CLOEXEC | O_NOFOLLOW | O_BINARY));
+  int flags = O_RDONLY | O_CLOEXEC | O_BINARY | (follow_symlinks ? 0 : O_NOFOLLOW);
+  int fd = TEMP_FAILURE_RETRY(open(path.c_str(), flags));
   if (fd == -1) {
     return false;
   }
@@ -91,8 +92,10 @@ static bool CleanUpAfterFailedWrite(const std::string& path) {
 
 #if !defined(_WIN32)
 bool WriteStringToFile(const std::string& content, const std::string& path,
-                       mode_t mode, uid_t owner, gid_t group) {
-  int flags = O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW | O_BINARY;
+                       mode_t mode, uid_t owner, gid_t group,
+                       bool follow_symlinks) {
+  int flags = O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_BINARY |
+              (follow_symlinks ? 0 : O_NOFOLLOW);
   int fd = TEMP_FAILURE_RETRY(open(path.c_str(), flags, mode));
   if (fd == -1) {
     PLOG(ERROR) << "android::WriteStringToFile open failed";
@@ -118,8 +121,10 @@ bool WriteStringToFile(const std::string& content, const std::string& path,
 }
 #endif
 
-bool WriteStringToFile(const std::string& content, const std::string& path) {
-  int flags = O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW | O_BINARY;
+bool WriteStringToFile(const std::string& content, const std::string& path,
+                       bool follow_symlinks) {
+  int flags = O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_BINARY |
+              (follow_symlinks ? 0 : O_NOFOLLOW);
   int fd = TEMP_FAILURE_RETRY(open(path.c_str(), flags, DEFFILEMODE));
   if (fd == -1) {
     return false;

base/file_test.cpp

@@ -45,6 +45,24 @@ TEST(file, ReadFileToString_WriteStringToFile) {
   EXPECT_EQ("abc", s);
 }
 
+// symlinks require elevated privileges on Windows.
+#if !defined(_WIN32)
+TEST(file, ReadFileToString_WriteStringToFile_symlink) {
+  TemporaryFile target, link;
+  ASSERT_EQ(0, unlink(link.path));
+  ASSERT_EQ(0, symlink(target.path, link.path));
+  ASSERT_FALSE(android::base::WriteStringToFile("foo", link.path, false));
+  ASSERT_EQ(ELOOP, errno);
+  ASSERT_TRUE(android::base::WriteStringToFile("foo", link.path, true));
+
+  std::string s;
+  ASSERT_FALSE(android::base::ReadFileToString(link.path, &s));
+  ASSERT_EQ(ELOOP, errno);
+  ASSERT_TRUE(android::base::ReadFileToString(link.path, &s, true));
+  ASSERT_EQ("foo", s);
+}
+#endif
+
 // WriteStringToFile2 is explicitly for setting Unix permissions, which make no
 // sense on Windows.
 #if !defined(_WIN32)

base/include/android-base/file.h

@@ -28,14 +28,17 @@ namespace android {
 namespace base {
 
 bool ReadFdToString(int fd, std::string* content);
-bool ReadFileToString(const std::string& path, std::string* content);
+bool ReadFileToString(const std::string& path, std::string* content,
+                      bool follow_symlinks = false);
 
-bool WriteStringToFile(const std::string& content, const std::string& path);
+bool WriteStringToFile(const std::string& content, const std::string& path,
+                       bool follow_symlinks = false);
 bool WriteStringToFd(const std::string& content, int fd);
 
 #if !defined(_WIN32)
 bool WriteStringToFile(const std::string& content, const std::string& path,
-                       mode_t mode, uid_t owner, gid_t group);
+                       mode_t mode, uid_t owner, gid_t group,
+                       bool follow_symlinks = false);
 #endif
 
 bool ReadFully(int fd, void* data, size_t byte_count);