Bug 485328: nsLocalFileUnix::GetNativeTarget() is broken for symlinks and (dangling) symlinks. r=neil, sr=bsmedberg

xpcom/io/nsLocalFileUnix.cpp

@@ -1476,16 +1476,13 @@ nsLocalFile::GetNativeTarget(nsACString &_retval)
     _retval.Truncate();
 
     struct stat symStat;
-    lstat(mPath.get(), &symStat);
+    if (lstat(mPath.get(), &symStat) == -1)
+        return NSRESULT_FOR_ERRNO();
+
     if (!S_ISLNK(symStat.st_mode))
         return NS_ERROR_FILE_INVALID_PATH;
 
-    PRInt64 targetSize64;
+    PRInt32 size = (PRInt32)symStat.st_size;
-    if (NS_FAILED(GetFileSizeOfLink(&targetSize64)))
-        return NS_ERROR_FAILURE;
-
-    PRInt32 size;
-    LL_L2I(size, targetSize64);
     char *target = (char *)nsMemory::Alloc(size + 1);
     if (!target)
         return NS_ERROR_OUT_OF_MEMORY;
@@ -1496,14 +1493,19 @@ nsLocalFile::GetNativeTarget(nsACString &_retval)
     }
     target[size] = '\0';
 
-    nsresult rv;
+    nsresult rv = NS_OK;
-    PRBool isSymlink;
     nsCOMPtr<nsIFile> self(this);
-    nsCOMPtr<nsIFile> parent;
+    PRInt32 maxLinks = 40;
-    while (NS_SUCCEEDED(rv = self->GetParent(getter_AddRefs(parent)))) {
+    while (PR_TRUE) {
-        NS_ASSERTION(parent != nsnull, "no parent?!");
+        if (maxLinks-- == 0) {
+            rv = NS_ERROR_FILE_UNRESOLVABLE_SYMLINK;
+            break;
+        }
 
         if (target[0] != '/') {
+            nsCOMPtr<nsIFile> parent;
+            if (NS_FAILED(rv = self->GetParent(getter_AddRefs(parent))))
+                break;
             nsCOMPtr<nsILocalFile> localFile(do_QueryInterface(parent, &rv));
             if (NS_FAILED(rv))
                 break;
@@ -1511,45 +1513,39 @@ nsLocalFile::GetNativeTarget(nsACString &_retval)
                 break;
             if (NS_FAILED(rv = localFile->GetNativePath(_retval)))
                 break;
-            if (NS_FAILED(rv = parent->IsSymlink(&isSymlink)))
-                break;
             self = parent;
         } else {
-            nsCOMPtr<nsILocalFile> localFile;
+            _retval = target;
-            rv = NS_NewNativeLocalFile(nsDependentCString(target), PR_TRUE,
-                                       getter_AddRefs(localFile));
-            if (NS_FAILED(rv))
-                break;
-            if (NS_FAILED(rv = localFile->IsSymlink(&isSymlink)))
-                break;
-            _retval = target; // XXX can we avoid this buffer copy?
-            self = do_QueryInterface(localFile);
         }
-        if (NS_FAILED(rv) || !isSymlink)
-            break;
 
         const nsPromiseFlatCString &flatRetval = PromiseFlatCString(_retval);
 
-        // strip off any and all trailing '/'
+        // Any failure in testing the current target we'll just interpret
-        PRInt32 len = strlen(target);
+        // as having reached our destiny.
-        while (target[len-1] == '/' && len > 1)
+        if (lstat(flatRetval.get(), &symStat) == -1)
-            target[--len] = '\0';
-        if (lstat(flatRetval.get(), &symStat) < 0) {
-            rv = NSRESULT_FOR_ERRNO();
             break;
-        }
-        if (!S_ISLNK(symStat.st_mode)) {
-            rv = NS_ERROR_FILE_INVALID_PATH;
-            break;
-        }
-        size = symStat.st_size;
-        if (readlink(flatRetval.get(), target, size) < 0) {
-            rv = NSRESULT_FOR_ERRNO();
-            break;
-        }
-        target[size] = '\0';
 
-        _retval.Truncate();
+        // And of course we're done if it isn't a symlink.
+        if (!S_ISLNK(symStat.st_mode))
+            break;
+
+        PRInt32 newSize = (PRInt32)symStat.st_size;
+        if (newSize > size) {
+            char *newTarget = (char *)nsMemory::Realloc(target, newSize + 1);
+            if (!newTarget) {
+                rv = NS_ERROR_OUT_OF_MEMORY;
+                break;
+            }
+            target = newTarget;
+            size = newSize;
+        }
+
+        PRInt32 linkLen = readlink(flatRetval.get(), target, size);
+        if (linkLen == -1) {
+            rv = NSRESULT_FOR_ERRNO();
+            break;
+        }
+        target[linkLen] = '\0';
     }
 
     nsMemory::Free(target);

xpcom/tests/unit/test_symlinks.js

@@ -56,20 +56,6 @@ function makeSymLink(from, toName, relative) {
   print("---");
   print(from.path);
   print(to.path);
-
-  if (isUnix && (from.leafName == DOES_NOT_EXIST || from.isSymlink())) {
-    // Bug 485328: nsLocalFileUnix::GetTarget() is broken for dangling symlinks
-    if (from.leafName != DOES_NOT_EXIST) {
-      let fromN = from.clone();
-      fromN.normalize();
-      let toN = to.clone();
-      toN.normalize();
-      do_check_eq(fromN.path, toN.path);
-    }
-
-    return to;
-  }
-
   print(to.target);
 
   if (isMac) {
@@ -128,9 +114,6 @@ function setupTestDir(testDir, relative) {
 
   makeSymLink(imaginary, DANGLING_LINK, relative);
 
-  if (isUnix) // Will be fixed in bug 485328
-    return;
-
   try {
     makeSymLink(loop, LOOP_LINK, relative);
     do_check_true(false);