Bug 156009 – run-webkit-tests must create parent directory of user's cache directory before running tests

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Daniel Bates]

The script run-webkit-tests influences the path chosen by a WebKit2 child process for the user's temporary directory and user's cache directory via the environment variable DIRHELPER_USER_DIR_SUFFIX. For the user's temporary directory, run-webkit-tests creates it as part of setting up the test environment. But run-webkit-tests does not create the user's cache directory. Therefore there is race between the time AppKit creates it and when the launched WebContent and Network processes query for the path to the user's cache directory such that the OS may return the empty string to the WebContent and Network processes if queried for this directory before AppKit creates it.

[Daniel Bates]

Created attachment 275182 [details]
Patch

[Daniel Bates]

Created attachment 275183 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 275183 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=275183&action=review

> Tools/Scripts/webkitpy/port/driver.py:136
> +        self._driver_user_directory_suffix = None
> +        self._driver_user_cache_directory = None

Maybe we should include the user directory for completeness ("/0/").

[Daniel Bates]

(In reply to comment #3)
> > Tools/Scripts/webkitpy/port/driver.py:136
> > +        self._driver_user_directory_suffix = None
> > +        self._driver_user_cache_directory = None
> 
> Maybe we should include the user directory for completeness ("/0/").

Filed bug #156025 to teach run-webkit-test to create and delete the directory _CS_DARWIN_USER_DIR.

[Daniel Bates]

(In reply to comment #0)
> [...] the OS may return the empty string to the WebContent and Network processes if queried for [the user's cache] directory before AppKit creates it.

Further elaborating on this, the WebContent (Network) process query the OS for the path to the user's cache directory with DIRHELPER_USER_DIR_SUFFIX of the form WebKitTestRunner-X/com.apple.WebKit.WebContent.Development (WebKitTestRunner-X/com.apple.WebKit.Networking.Development) by [1]. The processes query the OS via confstr(_CS_DARWIN_USER_CACHE_DIR, ..., ...). And confstr(_CS_DARWIN_USER_CACHE_DIR, ..., ...) will create the directory if it does not exist. At the time of writing, confstr(_CS_DARWIN_USER_CACHE_DIR, ..., ...) returns a path of the form $TMPDIR/C/$DIRHELPER_USER_DIR_SUFFIX where $TMPDIR and $DIRHELPER_USER_DIR_SUFFIX are the values of the environment variables of the same name. When DIRHELPER_USER_DIR_SUFFIX := WebKitTestRunner-X/com.apple.WebKit.WebContent.Development, confstr(_CS_DARWIN_USER_CACHE_DIR, ..., ...) will compute the path $TMPDIR/C/WebKitTestRunner-X/com.apple.WebKit.WebContent.Development and call mkdir() on it. Notice that mkdir() does not recursively create directory hierarchies. If $TMPDIR/C/WebKitTestRunner-X does not exist (because AppKit did not create it, yet) then mkdir($TMPDIR/C/WebKitTestRunner-X/com.apple.WebKit.WebContent.Development) will fail with errno ENOENT - "A component of the path prefix does not exist or path is an empty string." (MKDIR(2)) This failure causes a sandbox initialization error in the NetworkProcess by <http://trac.webkit.org/browser/trunk/Source/WebKit2/NetworkProcess/mac/com.apple.WebKit.NetworkProcess.sb.in?rev=197808#L77> because the empty string will be passed for parameter DARWIN_USER_CACHE_DIR.

[1] Disregarding the fact that the code has a correctness issue that prevents it from ever making use of such a suffix (we will fix this in bug #155455), the code that builds this DIRHELPER_USER_DIR_SUFFIX is at <http://trac.webkit.org/browser/trunk/Source/WebKit2/Shared/mac/ChildProcessMac.mm?rev=198237#L131>.

[Daniel Bates]

For completeness, Alexey Proskuryakov and I decided to fix this issue by teaching run-webkit-tests to create the user's cache directory instead of teaching the WebKit2 child process to create it to avoid adding filesystem code to WebKit2 with respect to the use of the value of the environment variable DIRHELPER_USER_DIR_SUFFIX. The value of this variable should never be used outside of WebKit's test infrastructure as it has security implications. We only make use of DIRHELPER_USER_DIR_SUFFIX when the UI process (client app) is not sandboxed as is the case for DumpRenderTree/WebKitTestRunner by <http://trac.webkit.org/browser/trunk/Source/WebKit2/Shared/EntryPointUtilities/mac/XPCService/XPCServiceEntryPoint.mm?rev=190257#L100>. We felt that adding code to WebKit2 to create the user's cache directory based on DIRHELPER_USER_DIR_SUFFIX could increase the attack surface should the WebKit2 be refactored without consideration of the security implications of DIRHELPER_USER_DIR_SUFFIX.

[Radar WebKit Bug Importer]

<rdar://problem/25442682>

[Daniel Bates]

Committed r198842: <http://trac.webkit.org/changeset/198842>