Attachment 368624 Details for Bug 197389 – Patch

[patch] Patch

bug-197389-20190430162225.patch (text/plain), 5.57 KB, created by Brent Fulgham on 2019-04-30 16:22:26 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Brent Fulgham

Created: 2019-04-30 16:22:26 PDT

Size: 5.57 KB

patch

obsolete

>Subversion Revision: 244755
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 9cfc789a2682e59163ea898ba7e4cafc337821ab..f0dc4fbb273373b05f7c91c6a78c811c5874b3cc 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,25 @@
>+2019-04-30  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 NOBODY (OOPS!).
>+
>+        The code in SandboxExtensionsCocoa.mm 'resolveSymlinksInPath' is pretty inefficient, and tries to reproduce (badly)
>+        logic that is already provided by the operating system.
>+
>+        Switch to using getattrlist with appropriate arguments to get the resolved actual paths without performing so many
>+        file operations.
>+
>+        To make matters worse, 'resolvePathForSandboxExtension' was effectively performing the work of fully resolving
>+        symlinks twice, since NSString's 'stringByStandardizingPath' method does this already.
>+
>+        * Shared/Cocoa/SandboxExtensionCocoa.mm:
>+        (WebKit::resolveSymlinksInPath): Update to use 'getattrlist'.
>+        (WebKit::resolvePathForSandboxExtension): Remove redundant call to 'resolveSymlinksInPath', since 'stringByStandardizingPath'
>+        already does this work.
>+
> 2019-04-29  Zalan Bujtas  <zalan@apple.com>
> 
>         [iOS] Double-tapping a post to like doesn't work on Instagram.com (needs 'dblclick' event)
>diff --git a/Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm b/Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm
>index c2ec473e9b0d28bb731c71136d8bf416ee4485db..5daa02c6c00e3aa5d7af2429c1e577eca26f65d9 100644
>--- a/Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm
>+++ b/Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm
>@@ -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,7 @@
> #import "DataReference.h"
> #import "Decoder.h"
> #import "Encoder.h"
>-#import <sys/stat.h>
>+#import <sys/attr.h>
> #import <wtf/FileSystem.h>
> #import <wtf/spi/darwin/SandboxSPI.h>
> #import <wtf/text/CString.h>
>@@ -227,45 +227,28 @@ RefPtr<SandboxExtension> SandboxExtension::create(Handle&& 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)
>+#pragma pack(push, 1)
>+    struct FullPathAttributeBuffer {
>+        uint32_t length;
>+        attrreference_t fullPathAttr;
>+        char fullPathBuf[PATH_MAX];
>+    };
>+#pragma pack(pop)
>+
>+    struct attrlist attrs = {
>+        .bitmapcount = ATTR_BIT_MAP_COUNT,
>+        .commonattr = ATTR_CMN_FULLPATH,
>+    };
>+    struct FullPathAttributeBuffer attrBuf;
>+
>+    if (getattrlist(path.data(), &attrs, &attrBuf, sizeof(attrBuf), FSOPT_ATTR_CMN_EXTENDED) < 0)
>         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);
>+    char* resolvedPathBuffer = (char*)&attrBuf.fullPathAttr + attrBuf.fullPathAttr.attr_dataoffset;
>+    uint32_t resolvedPathLength = attrBuf.fullPathAttr.attr_length - 1; // attr_length includes NULL-terminator byte.
>+    ASSERT(resolvedPathLength < PATH_MAX);
> 
>-    return resolvedPath;
>+    return CString(resolvedPathBuffer, resolvedPathLength);
> }
> 
> String stringByResolvingSymlinksInPath(const String& path)
>@@ -288,15 +271,13 @@ String resolveAndCreateReadWriteDirectoryForSandboxExtension(const String& path)
> 
> 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());
>         return { };
>     }
> 
>-    CString standardizedPath = resolveSymlinksInPath(fileSystemPath);
>-    return String::fromUTF8(standardizedPath);
>+    return String::fromUTF8(fileSystemPath);
> }
> 
> bool SandboxExtension::createHandleWithoutResolvingPath(const String& path, Type type, Handle& handle)

Actions: View | Formatted Diff | Diff

Attachments on bug 197389: 368496 | 368512 | 368519 | 368624 | 368806 | 368872 | 368961 | 369156