Changeset 244917 in webkit

Timestamp:

May 3, 2019 12:42:53 PM (5 years ago)

Author:

Brent Fulgham

Message:

Use more efficient path resolution logic
​https://bugs.webkit.org/show_bug.cgi?id=197389
<rdar://problem/50268491>

Reviewed by Maciej Stachowiak.

The code in SandboxExtensionsCocoa.mm 'resolveSymlinksInPath' is pretty inefficient, and tries to reproduce (badly)
logic that is already provided by the operating system.

To make matters worse, 'resolvePathForSandboxExtension' was effectively performing the work of fully resolving
symlinks twice, since NSString's 'stringByStandardizingPath' method does some of this already.

Instead, we should just use NSString's 'stringByResolvingSymlinksInPath', which does the symlink resolution
using more efficient logic than our 'resolveSymlinksInPath' code.

• Shared/Cocoa/SandboxExtensionCocoa.mm:

(WebKit::resolveSymlinksInPath): Removed.
(WebKit::resolvePathForSandboxExtension): Remove redundant call to 'resolveSymlinksInPath', and switches from
'stringByStandardizingPath' to 'stringByResolvingSymlinksInPath', which can take the place of both calls.
(WebKit::stringByResolvingSymlinksInPath): Switch to call 'stringByResolvingSymlinksInPath'.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Shared/Cocoa/SandboxExtensionCocoa.mm (modified) (4 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-05-03  Brent Fulgham  <bfulgham@apple.com>
+
+        Use more efficient path resolution logic
+        https://bugs.webkit.org/show_bug.cgi?id=197389
+        <rdar://problem/50268491>
+
+        Reviewed by Maciej Stachowiak.
+
+        The code in SandboxExtensionsCocoa.mm 'resolveSymlinksInPath' is pretty inefficient, and tries to reproduce (badly)
+        logic that is already provided by the operating system.
+
+        To make matters worse, 'resolvePathForSandboxExtension' was effectively performing the work of fully resolving
+        symlinks twice, since NSString's 'stringByStandardizingPath' method does some of this already.
+
+        Instead, we should just use NSString's 'stringByResolvingSymlinksInPath', which does the symlink resolution
+        using more efficient logic than our 'resolveSymlinksInPath' code.
+
+        * Shared/Cocoa/SandboxExtensionCocoa.mm:
+        (WebKit::resolveSymlinksInPath): Removed.
+        (WebKit::resolvePathForSandboxExtension): Remove redundant call to 'resolveSymlinksInPath', and switches from
+        'stringByStandardizingPath' to 'stringByResolvingSymlinksInPath', which can take the place of both calls.
+        (WebKit::stringByResolvingSymlinksInPath): Switch to call 'stringByResolvingSymlinksInPath'.
+
 2019-05-02  Dean Jackson  <dino@apple.com>
 

trunk/Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm

@@ -1 +1 @@
 /*
- * Copyright (C) 2010-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -32 +32 @@
 #import "Decoder.h"
 #import "Encoder.h"
-#import <sys/stat.h>
 #import <wtf/FileSystem.h>
 #import <wtf/spi/darwin/SandboxSPI.h>
@@ -226 +225 @@
 }
 
-static CString resolveSymlinksInPath(const CString& path)
-{
-    struct stat statBuf;
-
-    // Check if this file exists.
-    if (!stat(path.data(), &statBuf)) {
-        char resolvedName[PATH_MAX];
-
-        return realpath(path.data(), resolvedName);
-    }
-
-    const char* slashPtr = strrchr(path.data(), '/');
-    if (slashPtr == path.data())
-        return path;
-
-    size_t parentDirectoryLength = slashPtr - path.data();
-    if (parentDirectoryLength >= PATH_MAX)
-        return CString();
-
-    // Get the parent directory.
-    char parentDirectory[PATH_MAX];
-    memcpy(parentDirectory, path.data(), parentDirectoryLength);
-    parentDirectory[parentDirectoryLength] = '\0';
-
-    // Resolve it.
-    CString resolvedParentDirectory = resolveSymlinksInPath(CString(parentDirectory));
-    if (resolvedParentDirectory.isNull())
-        return CString();
-
-    size_t lastPathComponentLength = path.length() - parentDirectoryLength;
-    size_t resolvedPathLength = resolvedParentDirectory.length() + lastPathComponentLength;
-    if (resolvedPathLength >= PATH_MAX)
-        return CString();
-
-    // Combine the resolved parent directory with the last path component.
-    char* resolvedPathBuffer;
-    CString resolvedPath = CString::newUninitialized(resolvedPathLength, resolvedPathBuffer);
-    memcpy(resolvedPathBuffer, resolvedParentDirectory.data(), resolvedParentDirectory.length());
-    memcpy(resolvedPathBuffer + resolvedParentDirectory.length(), slashPtr, lastPathComponentLength);
-
-    return resolvedPath;
-}
-
 String stringByResolvingSymlinksInPath(const String& path)
 {
-    return String::fromUTF8(resolveSymlinksInPath(path.utf8()));
+    return [(NSString *)path stringByResolvingSymlinksInPath];
 }
 
@@ -289 +245 @@
 String resolvePathForSandboxExtension(const String& path)
 {
-    // FIXME: Do we need both resolveSymlinksInPath() and -stringByStandardizingPath?
-    CString fileSystemPath = FileSystem::fileSystemRepresentation([(NSString *)path stringByStandardizingPath]);
-    if (fileSystemPath.isNull()) {
-        LOG_ERROR("Could not create a valid file system representation for the string '%s' of length %lu", fileSystemPath.data(), fileSystemPath.length());
+    String resolvedPath = stringByResolvingSymlinksInPath(path);
+    if (resolvedPath.isEmpty()) {
+        LOG_ERROR("Could not create a valid file system representation for the string '%s' of length %lu", resolvedPath.utf8().data(), resolvedPath.length());
         return { };
     }
 
-    CString standardizedPath = resolveSymlinksInPath(fileSystemPath);
-    return String::fromUTF8(standardizedPath);
+    return resolvedPath;
 }