Diff
Modified: trunk/Source/WebKit/ChangeLog (242291 => 242292)
--- trunk/Source/WebKit/ChangeLog 2019-03-01 23:09:31 UTC (rev 242291)
+++ trunk/Source/WebKit/ChangeLog 2019-03-01 23:30:16 UTC (rev 242292)
@@ -1,3 +1,31 @@
+2019-03-01 Ross Kirsling <ross.kirsl...@sony.com>
+
+ EnvironmentUtilities::stripValuesEndingWithString isn't thread-safe
+ https://bugs.webkit.org/show_bug.cgi?id=194612
+
+ Reviewed by Alex Christensen.
+
+ This API test really shouldn't be verifying that the actual environment was successfully modified.
+
+ At its core, stripValuesEndingWithString is really just split-filter-join. By replacing it with a pair of
+ simple functions -- one for string processing, one for environment processing -- the API test only needs to
+ worry about the former.
+
+ * Platform/unix/EnvironmentUtilities.cpp:
+ (WebKit::EnvironmentUtilities::stripEntriesEndingWith):
+ (WebKit::EnvironmentUtilities::removeValuesEndingWith):
+ (WebKit::EnvironmentUtilities::stripValuesEndingWithString): Deleted.
+ * Platform/unix/EnvironmentUtilities.h:
+ Replace old function with a pair of simpler ones.
+
+ * NetworkProcess/EntryPoint/Cocoa/XPCService/NetworkServiceEntryPoint.mm:
+ (NetworkServiceInitializer):
+ * PluginProcess/EntryPoint/Cocoa/XPCService/PluginServiceEntryPoint.mm:
+ (PluginServiceInitializer):
+ * WebProcess/EntryPoint/Cocoa/XPCService/WebContentServiceEntryPoint.mm:
+ (WebContentServiceInitializer):
+ Update function name.
+
2019-03-01 Don Olmstead <don.olmst...@sony.com>
Unify WebsiteDataStore::defaultDataStoreConfiguration across ports
Modified: trunk/Source/WebKit/NetworkProcess/EntryPoint/Cocoa/XPCService/NetworkServiceEntryPoint.mm (242291 => 242292)
--- trunk/Source/WebKit/NetworkProcess/EntryPoint/Cocoa/XPCService/NetworkServiceEntryPoint.mm 2019-03-01 23:09:31 UTC (rev 242291)
+++ trunk/Source/WebKit/NetworkProcess/EntryPoint/Cocoa/XPCService/NetworkServiceEntryPoint.mm 2019-03-01 23:30:16 UTC (rev 242292)
@@ -56,6 +56,6 @@
{
// Remove the SecItemShim from the DYLD_INSERT_LIBRARIES environment variable so any processes spawned by
// the this process don't try to insert the shim and crash.
- EnvironmentUtilities::stripValuesEndingWithString("DYLD_INSERT_LIBRARIES", "/SecItemShim.dylib");
+ EnvironmentUtilities::removeValuesEndingWith("DYLD_INSERT_LIBRARIES", "/SecItemShim.dylib");
XPCServiceInitializer<NetworkProcess, NetworkServiceInitializerDelegate>(adoptOSObject(connection), initializerMessage, priorityBoostMessage);
}
Modified: trunk/Source/WebKit/Platform/unix/EnvironmentUtilities.cpp (242291 => 242292)
--- trunk/Source/WebKit/Platform/unix/EnvironmentUtilities.cpp 2019-03-01 23:09:31 UTC (rev 242291)
+++ trunk/Source/WebKit/Platform/unix/EnvironmentUtilities.cpp 2019-03-01 23:30:16 UTC (rev 242292)
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2019 Sony Interactive Entertainment Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -26,104 +27,46 @@
#include "config.h"
#include "EnvironmentUtilities.h"
-#include <wtf/text/CString.h>
+#include <cstdlib>
+#include <wtf/text/StringBuilder.h>
namespace WebKit {
namespace EnvironmentUtilities {
-void stripValuesEndingWithString(const char* environmentVariable, const char* searchValue)
+String stripEntriesEndingWith(StringView input, StringView suffix)
{
- ASSERT(environmentVariable);
- ASSERT(searchValue);
-
- // Grab the current value of the environment variable.
- char* environmentValue = getenv(environmentVariable);
+ StringBuilder output;
- if (!environmentValue || environmentValue[0] == '\0')
- return;
+ auto hasAppended = false;
+ for (auto entry : input.splitAllowingEmptyEntries(':')) {
+ if (entry.endsWith(suffix))
+ continue;
- const size_t environmentValueLength = strlen(environmentValue);
- const size_t environmentValueBufferLength = environmentValueLength + 1;
+ if (hasAppended)
+ output.append(':');
+ else
+ hasAppended = true;
- // Set up the strings we'll be searching for.
- size_t searchLength = strlen(searchValue);
- if (!searchLength)
- return;
-
- Vector<char> searchValueWithColonVector;
- searchValueWithColonVector.grow(searchLength + 2);
- char* searchValueWithColon = searchValueWithColonVector.data();
- size_t searchLengthWithColon = searchLength + 1;
-
- memcpy(searchValueWithColon, searchValue, searchLength);
- searchValueWithColon[searchLength] = ':';
- searchValueWithColon[searchLengthWithColon] = '\0';
-
- // Loop over environmentValueBuffer, removing any components that match the search value ending with a colon.
- char* componentStart = environmentValue;
- char* match = strnstr(componentStart, searchValueWithColon, environmentValueLength - static_cast<size_t>(componentStart - environmentValue));
- bool foundAnyMatches = match != NULL;
- while (match != NULL) {
- // Update componentStart to point to the colon immediately preceding the match.
- char* nextColon = strnstr(componentStart, ":", environmentValueLength - static_cast<size_t>(componentStart - environmentValue));
- while (nextColon && nextColon < match) {
- componentStart = nextColon;
- nextColon = strnstr(componentStart + 1, ":", environmentValueLength - static_cast<size_t>(componentStart + 1 - environmentValue));
- }
-
- RELEASE_ASSERT(componentStart >= environmentValue);
- size_t environmentValueOffset = static_cast<size_t>(componentStart - environmentValue);
- RELEASE_ASSERT(environmentValueOffset < environmentValueBufferLength);
-
- // Copy over everything right of the match to the current component start, and search from there again.
- if (componentStart[0] == ':') {
- // If componentStart points to a colon, copy the colon over.
- strlcpy(componentStart, match + searchLength, environmentValueBufferLength - environmentValueOffset);
- } else {
- // Otherwise, componentStart still points to the beginning of environmentValueBuffer, so don't copy over the colon.
- // The edge case is if the colon is the last character in the string, so "match + searchLengthWithoutColon + 1" is the
- // null terminator of the original input, in which case this is still safe.
- strlcpy(componentStart, match + searchLengthWithColon, environmentValueBufferLength - environmentValueOffset);
- }
-
- match = strnstr(componentStart, searchValueWithColon, environmentValueLength - static_cast<size_t>(componentStart - environmentValue));
+ output.append(entry);
}
-
- // Search for the value without a trailing colon, seeing if the original input ends with it.
- match = strnstr(componentStart, searchValue, environmentValueLength - static_cast<size_t>(componentStart - environmentValue));
- while (match != NULL) {
- if (match[searchLength] == '\0')
- break;
- match = strnstr(match + 1, searchValue, environmentValueLength - static_cast<size_t>(match + 1 - environmentValue));
- }
-
- // Since the original input ends with the search, strip out the last component.
- if (match) {
- // Update componentStart to point to the colon immediately preceding the match.
- char* nextColon = strnstr(componentStart, ":", environmentValueLength - static_cast<size_t>(componentStart - environmentValue));
- while (nextColon && nextColon < match) {
- componentStart = nextColon;
- nextColon = strnstr(componentStart + 1, ":", environmentValueLength - static_cast<size_t>(componentStart + 1 - environmentValue));
- }
-
- // Whether componentStart points to the original string or the last colon, putting the null terminator there will get us the desired result.
- componentStart[0] = '\0';
- foundAnyMatches = true;
- }
+ return output.toString();
+}
- // If we found no matches, don't change anything.
- if (!foundAnyMatches)
+void removeValuesEndingWith(const char* environmentVariable, const char* searchValue)
+{
+ const char* before = getenv(environmentVariable);
+ if (!before)
return;
- // If we have nothing left, just unset the variable
- if (environmentValue[0] == '\0') {
+ auto after = stripEntriesEndingWith(before, searchValue);
+ if (after.isEmpty()) {
unsetenv(environmentVariable);
return;
}
-
- setenv(environmentVariable, environmentValue, 1);
+
+ setenv(environmentVariable, after.utf8().data(), 1);
}
} // namespace EnvironmentUtilities
Modified: trunk/Source/WebKit/Platform/unix/EnvironmentUtilities.h (242291 => 242292)
--- trunk/Source/WebKit/Platform/unix/EnvironmentUtilities.h 2019-03-01 23:09:31 UTC (rev 242291)
+++ trunk/Source/WebKit/Platform/unix/EnvironmentUtilities.h 2019-03-01 23:30:16 UTC (rev 242292)
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2019 Sony Interactive Entertainment Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -26,6 +27,7 @@
#pragma once
#include "WKDeclarationSpecifiers.h"
+#include <wtf/text/StringView.h>
#include <wtf/text/WTFString.h>
namespace WebKit {
@@ -32,7 +34,8 @@
namespace EnvironmentUtilities {
-WK_EXPORT void stripValuesEndingWithString(const char* environmentVariable, const char* search);
+WK_EXPORT String stripEntriesEndingWith(StringView input, StringView suffix);
+WK_EXPORT void removeValuesEndingWith(const char* environmentVariable, const char* search);
} // namespace EnvironmentUtilities
Modified: trunk/Source/WebKit/PluginProcess/EntryPoint/Cocoa/XPCService/PluginServiceEntryPoint.mm (242291 => 242292)
--- trunk/Source/WebKit/PluginProcess/EntryPoint/Cocoa/XPCService/PluginServiceEntryPoint.mm 2019-03-01 23:09:31 UTC (rev 242291)
+++ trunk/Source/WebKit/PluginProcess/EntryPoint/Cocoa/XPCService/PluginServiceEntryPoint.mm 2019-03-01 23:30:16 UTC (rev 242292)
@@ -78,7 +78,7 @@
// Remove the PluginProcess shim from the DYLD_INSERT_LIBRARIES environment variable so any processes
// spawned by the PluginProcess don't try to insert the shim and crash.
- EnvironmentUtilities::stripValuesEndingWithString("DYLD_INSERT_LIBRARIES", "/PluginProcessShim.dylib");
+ EnvironmentUtilities::removeValuesEndingWith("DYLD_INSERT_LIBRARIES", "/PluginProcessShim.dylib");
XPCServiceInitializer<PluginProcess, PluginServiceInitializerDelegate>(adoptOSObject(connection), initializerMessage, priorityBoostMessage);
#endif // ENABLE(NETSCAPE_PLUGIN_API)
}
Modified: trunk/Source/WebKit/WebProcess/EntryPoint/Cocoa/XPCService/WebContentServiceEntryPoint.mm (242291 => 242292)
--- trunk/Source/WebKit/WebProcess/EntryPoint/Cocoa/XPCService/WebContentServiceEntryPoint.mm 2019-03-01 23:09:31 UTC (rev 242291)
+++ trunk/Source/WebKit/WebProcess/EntryPoint/Cocoa/XPCService/WebContentServiceEntryPoint.mm 2019-03-01 23:30:16 UTC (rev 242292)
@@ -41,7 +41,7 @@
{
// Remove the WebProcessShim from the DYLD_INSERT_LIBRARIES environment variable so any processes spawned by
// the this process don't try to insert the shim and crash.
- WebKit::EnvironmentUtilities::stripValuesEndingWithString("DYLD_INSERT_LIBRARIES", "/WebProcessShim.dylib");
+ WebKit::EnvironmentUtilities::removeValuesEndingWith("DYLD_INSERT_LIBRARIES", "/WebProcessShim.dylib");
#if PLATFORM(IOS_FAMILY)
GSInitialize();
Modified: trunk/Tools/ChangeLog (242291 => 242292)
--- trunk/Tools/ChangeLog 2019-03-01 23:09:31 UTC (rev 242291)
+++ trunk/Tools/ChangeLog 2019-03-01 23:30:16 UTC (rev 242292)
@@ -1,3 +1,14 @@
+2019-03-01 Ross Kirsling <ross.kirsl...@sony.com>
+
+ EnvironmentUtilities::stripValuesEndingWithString isn't thread-safe
+ https://bugs.webkit.org/show_bug.cgi?id=194612
+
+ Reviewed by Alex Christensen.
+
+ * TestWebKitAPI/Tests/WebKit/EnvironmentUtilitiesTest.cpp:
+ Just test the new string-processing function and don't touch the actual environment.
+ (Test cases are all as before, but based on operator== instead of strcmp.)
+
2019-03-01 Aakash Jain <aakash_j...@apple.com>
[ews-app] Update primary keys for handling multiple Buildbot instances
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit/EnvironmentUtilitiesTest.cpp (242291 => 242292)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit/EnvironmentUtilitiesTest.cpp 2019-03-01 23:09:31 UTC (rev 242291)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit/EnvironmentUtilitiesTest.cpp 2019-03-01 23:30:16 UTC (rev 242292)
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2019 Sony Interactive Entertainment Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -27,60 +28,56 @@
#include "Test.h"
#include <WebKit/EnvironmentUtilities.h>
-#include <stdlib.h>
namespace TestWebKitAPI {
-const char* const environmentVariable = "DYLD_INSERT_LIBRARIES";
#define PROCESS_DYLIB "Process.dylib"
const char* const stripValue = "/" PROCESS_DYLIB;
-static const char* strip(const char* input)
+static String strip(StringView input)
{
- setenv(environmentVariable, input, 1);
- WebKit::EnvironmentUtilities::stripValuesEndingWithString(environmentVariable, stripValue);
- return getenv(environmentVariable);
+ return WebKit::EnvironmentUtilities::stripEntriesEndingWith(input, stripValue);
}
-TEST(WebKit, StripValuesEndingWithString)
+TEST(WebKit, StripEntriesEndingWith)
{
- EXPECT_STREQ(strip(""), "");
- EXPECT_STREQ(strip(":"), ":");
- EXPECT_STREQ(strip("::"), "::");
- EXPECT_STREQ(strip(":::"), ":::");
- EXPECT_STREQ(strip("::::"), "::::");
- EXPECT_STREQ(strip(":::::"), ":::::");
+ EXPECT_EQ(strip(""), "");
+ EXPECT_EQ(strip(":"), ":");
+ EXPECT_EQ(strip("::"), "::");
+ EXPECT_EQ(strip(":::"), ":::");
+ EXPECT_EQ(strip("::::"), "::::");
+ EXPECT_EQ(strip(":::::"), ":::::");
- EXPECT_STREQ(strip(PROCESS_DYLIB), PROCESS_DYLIB);
- EXPECT_STREQ(strip(":" PROCESS_DYLIB), ":" PROCESS_DYLIB);
- EXPECT_STREQ(strip(PROCESS_DYLIB ":"), PROCESS_DYLIB ":");
- EXPECT_STREQ(strip(":" PROCESS_DYLIB ":"), ":" PROCESS_DYLIB ":");
+ EXPECT_EQ(strip(PROCESS_DYLIB), PROCESS_DYLIB);
+ EXPECT_EQ(strip(":" PROCESS_DYLIB), ":" PROCESS_DYLIB);
+ EXPECT_EQ(strip(PROCESS_DYLIB ":"), PROCESS_DYLIB ":");
+ EXPECT_EQ(strip(":" PROCESS_DYLIB ":"), ":" PROCESS_DYLIB ":");
- EXPECT_STREQ(strip("/" PROCESS_DYLIB), nullptr);
- EXPECT_STREQ(strip(":/" PROCESS_DYLIB), nullptr);
- EXPECT_STREQ(strip("/" PROCESS_DYLIB ":"), nullptr);
- EXPECT_STREQ(strip(":/" PROCESS_DYLIB ":"), ":");
+ EXPECT_EQ(strip("/" PROCESS_DYLIB), "");
+ EXPECT_EQ(strip(":/" PROCESS_DYLIB), "");
+ EXPECT_EQ(strip("/" PROCESS_DYLIB ":"), "");
+ EXPECT_EQ(strip(":/" PROCESS_DYLIB ":"), ":");
- EXPECT_STREQ(strip(PROCESS_DYLIB "/"), PROCESS_DYLIB "/");
- EXPECT_STREQ(strip(":" PROCESS_DYLIB "/"), ":" PROCESS_DYLIB "/");
- EXPECT_STREQ(strip(PROCESS_DYLIB "/:"), PROCESS_DYLIB "/:");
- EXPECT_STREQ(strip(":" PROCESS_DYLIB "/:"), ":" PROCESS_DYLIB "/:");
+ EXPECT_EQ(strip(PROCESS_DYLIB "/"), PROCESS_DYLIB "/");
+ EXPECT_EQ(strip(":" PROCESS_DYLIB "/"), ":" PROCESS_DYLIB "/");
+ EXPECT_EQ(strip(PROCESS_DYLIB "/:"), PROCESS_DYLIB "/:");
+ EXPECT_EQ(strip(":" PROCESS_DYLIB "/:"), ":" PROCESS_DYLIB "/:");
- EXPECT_STREQ(strip("/" PROCESS_DYLIB "/"), "/" PROCESS_DYLIB "/");
- EXPECT_STREQ(strip(":/" PROCESS_DYLIB "/"), ":/" PROCESS_DYLIB "/");
- EXPECT_STREQ(strip("/" PROCESS_DYLIB "/:"), "/" PROCESS_DYLIB "/:");
- EXPECT_STREQ(strip(":/" PROCESS_DYLIB "/:"), ":/" PROCESS_DYLIB "/:");
+ EXPECT_EQ(strip("/" PROCESS_DYLIB "/"), "/" PROCESS_DYLIB "/");
+ EXPECT_EQ(strip(":/" PROCESS_DYLIB "/"), ":/" PROCESS_DYLIB "/");
+ EXPECT_EQ(strip("/" PROCESS_DYLIB "/:"), "/" PROCESS_DYLIB "/:");
+ EXPECT_EQ(strip(":/" PROCESS_DYLIB "/:"), ":/" PROCESS_DYLIB "/:");
- EXPECT_STREQ(strip("/Before.dylib:/" PROCESS_DYLIB), "/Before.dylib");
- EXPECT_STREQ(strip("/" PROCESS_DYLIB ":/After.dylib"), "/After.dylib");
- EXPECT_STREQ(strip("/Before.dylib:/" PROCESS_DYLIB ":/After.dylib"), "/Before.dylib:/After.dylib");
- EXPECT_STREQ(strip("/Before.dylib:/" PROCESS_DYLIB ":/Middle.dylib:/" PROCESS_DYLIB ":/After.dylib"), "/Before.dylib:/Middle.dylib:/After.dylib");
+ EXPECT_EQ(strip("/Before.dylib:/" PROCESS_DYLIB), "/Before.dylib");
+ EXPECT_EQ(strip("/" PROCESS_DYLIB ":/After.dylib"), "/After.dylib");
+ EXPECT_EQ(strip("/Before.dylib:/" PROCESS_DYLIB ":/After.dylib"), "/Before.dylib:/After.dylib");
+ EXPECT_EQ(strip("/Before.dylib:/" PROCESS_DYLIB ":/Middle.dylib:/" PROCESS_DYLIB ":/After.dylib"), "/Before.dylib:/Middle.dylib:/After.dylib");
- EXPECT_STREQ(strip("/" PROCESS_DYLIB ":/" PROCESS_DYLIB), nullptr);
- EXPECT_STREQ(strip("/" PROCESS_DYLIB ":/" PROCESS_DYLIB ":/" PROCESS_DYLIB), nullptr);
+ EXPECT_EQ(strip("/" PROCESS_DYLIB ":/" PROCESS_DYLIB), "");
+ EXPECT_EQ(strip("/" PROCESS_DYLIB ":/" PROCESS_DYLIB ":/" PROCESS_DYLIB), "");
- EXPECT_STREQ(strip("/usr/lib/" PROCESS_DYLIB), nullptr);
- EXPECT_STREQ(strip("/" PROCESS_DYLIB "/" PROCESS_DYLIB), nullptr);
+ EXPECT_EQ(strip("/usr/lib/" PROCESS_DYLIB), "");
+ EXPECT_EQ(strip("/" PROCESS_DYLIB "/" PROCESS_DYLIB), "");
}
}
