Title: [244969] trunk/Source/WebKit
Revision
244969
Author
bfulg...@apple.com
Date
2019-05-06 13:14:10 -0700 (Mon, 06 May 2019)

Log 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'.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (244968 => 244969)


--- trunk/Source/WebKit/ChangeLog	2019-05-06 18:32:26 UTC (rev 244968)
+++ trunk/Source/WebKit/ChangeLog	2019-05-06 20:14:10 UTC (rev 244969)
@@ -1,3 +1,26 @@
+2019-05-06  Brent Fulgham  <bfulg...@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-06  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Introduce SPI to request modern compatibility mode but defer to site-specific quirks

Modified: trunk/Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm (244968 => 244969)


--- trunk/Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm	2019-05-06 18:32:26 UTC (rev 244968)
+++ trunk/Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm	2019-05-06 20:14:10 UTC (rev 244969)
@@ -1,5 +1,5 @@
 /*
- * 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
  * modification, are permitted provided that the following conditions
@@ -31,7 +31,6 @@
 #import "DataReference.h"
 #import "Decoder.h"
 #import "Encoder.h"
-#import <sys/stat.h>
 #import <wtf/FileSystem.h>
 #import <wtf/spi/darwin/SandboxSPI.h>
 #import <wtf/text/CString.h>
@@ -225,52 +224,9 @@
     return adoptRef(new SandboxExtension(handle));
 }
 
-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];
 }
 
 String resolveAndCreateReadWriteDirectoryForSandboxExtension(const String& path)
@@ -288,15 +244,13 @@
 
 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.isNull()) {
+        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;
 }
 
 bool SandboxExtension::createHandleWithoutResolvingPath(const String& path, Type type, Handle& handle)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to