Skip to content

Commit

Permalink
Ghostery throws exception trying to use storageArea.set().
Browse files Browse the repository at this point in the history
https://webkit.org/b/268565
rdar://problem/122118152

Reviewed by Jeff Miller.

We need to use JavaScript's native JSON encoder for storage, since extensions like Ghostery
use toJSON() functions on custom objects to set data in storage.

Teach the code generator how to convert only the top-level keys of a dictionary while
having all the values be JSValue. This allows for the convenience of a dictionary for
the keys, and true values for conversion with JavaScript's JSON stringifier.

* Source/WebKit/Platform/cocoa/CocoaHelpers.mm:
(WebKit::encodeJSONString): Use JSValue's _toJSONString when it is a JSValue.
(WebKit::encodeJSONData): Ditto.
* Source/WebKit/Shared/Extensions/WebExtensionUtilities.h:
* Source/WebKit/Shared/Extensions/WebExtensionUtilities.mm:
(WebKit::anyItemsExceedQuota): Added keyWithError out param for better error logging.
* Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIStorageAreaCocoa.mm:
(WebKit::WebExtensionAPIStorageArea::set): Record JSON/quota errors per key for debugging ease.
* Source/WebKit/WebProcess/Extensions/Bindings/Cocoa/JSWebExtensionWrapperCocoa.mm:
(WebKit::toNSDictionary): Added ValuePolicy key to stop at top-level.
* Source/WebKit/WebProcess/Extensions/Bindings/JSWebExtensionWrapper.h:
* Source/WebKit/WebProcess/Extensions/Bindings/Scripts/CodeGeneratorExtensions.pm:
(_platformTypeConstructor): Added support for NSDictionary=StopAtTopLevel attribute.
* Source/WebKit/WebProcess/Extensions/Interfaces/WebExtensionAPIStorageArea.idl:
(set): Use NSDictionary=StopAtTopLevel for items.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIStorage.mm:
(TestWebKitAPI::TEST): Added new test, and also test quota and null.

Canonical link: https://commits.webkit.org/273949@main
  • Loading branch information
xeenon committed Feb 1, 2024
1 parent 7df0ea2 commit 944eaf1
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 23 deletions.
20 changes: 19 additions & 1 deletion Source/WebKit/Platform/cocoa/CocoaHelpers.mm
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,13 @@
#endif

#import "config.h"
#import "APIData.h"
#import "CocoaHelpers.h"

#import "APIData.h"
#import "JSWebExtensionWrapper.h"
#import "Logging.h"
#import "WKNSData.h"
#import <JavaScriptCore/JSValue.h>
#import <wtf/FileSystem.h>

namespace WebKit {
Expand Down Expand Up @@ -267,8 +270,16 @@ id parseJSON(API::Data& json, JSONOptionSet options, NSError **error)

NSString *encodeJSONString(id object, JSONOptionSet options, NSError **error)
{
if (JSValue *value = dynamic_objc_cast<JSValue>(object)) {
if (!options.contains(JSONOptions::FragmentsAllowed) && !value._isDictionary)
return nil;

return value._toJSONString;
}

if (auto *data = encodeJSONData(object, options, error))
return [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding];

return nil;
}

Expand All @@ -277,6 +288,13 @@ id parseJSON(API::Data& json, JSONOptionSet options, NSError **error)
if (!object)
return nil;

if (JSValue *value = dynamic_objc_cast<JSValue>(object)) {
if (!options.contains(JSONOptions::FragmentsAllowed) && !value._isDictionary)
return nil;

return [value._toJSONString dataUsingEncoding:NSUTF8StringEncoding];
}

ASSERT(isValidJSONObject(object, options));

if (!options.contains(JSONOptions::FragmentsAllowed) && ![object isKindOfClass:NSDictionary.class])
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/Shared/Extensions/WebExtensionUtilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ size_t storageSizeOf(NSString *);
size_t storageSizeOf(NSDictionary<NSString *, NSString *> *);

/// Returns true if the size of any item in the dictionary exceeds the given quota.
bool anyItemsExceedQuota(NSDictionary *, size_t quota);
bool anyItemsExceedQuota(NSDictionary *, size_t quota, NSString **outKeyWithError = nullptr);

} // namespace WebKit

Expand Down
14 changes: 11 additions & 3 deletions Source/WebKit/Shared/Extensions/WebExtensionUtilities.mm
Original file line number Diff line number Diff line change
Expand Up @@ -387,17 +387,25 @@ size_t storageSizeOf(NSDictionary<NSString *, NSString *> *keysAndValues)
return storageSize;
}

bool anyItemsExceedQuota(NSDictionary *items, size_t quota)
bool anyItemsExceedQuota(NSDictionary *items, size_t quota, NSString **outKeyWithError)
{
__block BOOL itemExceededQuota = NO;
__block bool itemExceededQuota;
__block NSString *keyWithError;

[items enumerateKeysAndObjectsUsingBlock:^(NSString *key, NSString *value, BOOL *stop) {
size_t sizeOfCurrentItem = storageSizeOf(key) + storageSizeOf(value);
if (sizeOfCurrentItem > quota) {
itemExceededQuota = YES;
itemExceededQuota = true;
keyWithError = key;
*stop = YES;
}
}];

ASSERT(!itemExceededQuota || (itemExceededQuota && keyWithError));

if (outKeyWithError)
*outKeyWithError = keyWithError;

return itemExceededQuota;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,17 +152,30 @@
return;
}

if (!isValidJSONObject(items)) {
*outExceptionString = toErrorString(nil, @"items", @"it is not JSON-serializable");
return;
}
__block NSString *keyWithError;

auto *serializedData = mapObjects(items, ^NSString *(NSString *key, id object) {
ASSERT([object isKindOfClass:JSValue.class]);

NSDictionary<NSString *, NSString *> *serializedData = mapObjects(items, ^NSString *(NSString *key, id object) {
return encodeJSONString(object, { JSONOptions::FragmentsAllowed });
if (keyWithError)
return nil;

auto *result = encodeJSONString(object, JSONOptions::FragmentsAllowed);
if (!result) {
keyWithError = key;
return nil;
}

return result;
});

if (anyItemsExceedQuota(serializedData, webExtensionStorageAreaSyncQuotaBytesPerItem)) {
*outExceptionString = toErrorString(nil, @"items", @"exceeded maximum size for a single item");
if (keyWithError) {
*outExceptionString = toErrorString(nil, [NSString stringWithFormat:@"items[`%@`]", keyWithError], @"it is not JSON-serializable");
return;
}

if (anyItemsExceedQuota(serializedData, webExtensionStorageAreaSyncQuotaBytesPerItem, &keyWithError)) {
*outExceptionString = toErrorString(nil, [NSString stringWithFormat:@"items[`%@`]", keyWithError], @"exceeded maximum size for a single item");
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ id toNSObject(JSContextRef context, JSValueRef valueRef, Class containingObjects
}
}

NSDictionary *toNSDictionary(JSContextRef context, JSValueRef valueRef, NullValuePolicy nullPolicy)
NSDictionary *toNSDictionary(JSContextRef context, JSValueRef valueRef, NullValuePolicy nullPolicy, ValuePolicy valuePolicy)
{
ASSERT(context);

Expand Down Expand Up @@ -248,12 +248,20 @@ id toNSObject(JSContextRef context, JSValueRef valueRef, Class containingObjects
if (nullPolicy == NullValuePolicy::NotAllowed && JSValueIsNull(context, item))
continue;

auto *key = toNSString(propertyName.get());
auto *itemValue = toJSValue(context, item);

if (valuePolicy == ValuePolicy::StopAtTopLevel) {
if (itemValue)
result[key] = itemValue;
continue;
}

if (itemValue._isDictionary) {
if (auto *itemDictionary = toNSDictionary(context, item, nullPolicy))
[result setObject:itemDictionary forKey:toNSString(propertyName.get())];
result[key] = itemDictionary;
} else if (id value = toNSObject(context, item))
[result setObject:value forKey:toNSString(propertyName.get())];
result[key] = value;
}

JSPropertyNameArrayRelease(propertyNames);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ enum class NullValuePolicy : bool {
Allowed,
};

enum class ValuePolicy : bool {
Recursive,
StopAtTopLevel,
};

inline RefPtr<WebFrame> toWebFrame(JSContextRef context)
{
ASSERT(context);
Expand Down Expand Up @@ -167,7 +172,7 @@ RefPtr<WebExtensionCallbackHandler> toJSCallbackHandler(JSContextRef, JSValueRef

id toNSObject(JSContextRef, JSValueRef, Class containingObjectsOfClass = Nil);
NSString *toNSString(JSContextRef, JSValueRef, NullStringPolicy = NullStringPolicy::NullAndUndefinedAsNullString);
NSDictionary *toNSDictionary(JSContextRef, JSValueRef, NullValuePolicy = NullValuePolicy::NotAllowed);
NSDictionary *toNSDictionary(JSContextRef, JSValueRef, NullValuePolicy = NullValuePolicy::NotAllowed, ValuePolicy = ValuePolicy::Recursive);

inline NSArray *toNSArray(JSContextRef context, JSValueRef value, Class containingObjectsOfClass = NSObject.class)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,7 @@ sub _platformTypeConstructor

if ($idlTypeName eq "any") {
return "serializeJSObject(context, $argumentName, exception)" if $signature->extendedAttributes->{"Serialization"} && $signature->extendedAttributes->{"Serialization"} eq "JSON";
return "toNSDictionary(context, $argumentName, NullValuePolicy::Allowed, ValuePolicy::StopAtTopLevel)" if $signature->extendedAttributes->{"NSDictionary"} && $signature->extendedAttributes->{"NSDictionary"} eq "StopAtTopLevel";
return "toNSDictionary(context, $argumentName, NullValuePolicy::Allowed)" if $signature->extendedAttributes->{"NSDictionary"} && $signature->extendedAttributes->{"NSDictionary"} eq "NullAllowed";
return "toNSDictionary(context, $argumentName, NullValuePolicy::NotAllowed)" if $signature->extendedAttributes->{"NSDictionary"};
return "toNSArray(context, $argumentName, $arrayType.class)" if $signature->extendedAttributes->{"NSArray"} && $arrayType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

[RaisesException] void get([Optional, NSObject, DOMString] any items, [Optional, CallbackHandler] function callback);
[RaisesException] void getBytesInUse([Optional, NSObject, DOMString] any keys, [Optional, CallbackHandler] function callback);
[RaisesException] void set([NSDictionary] any items, [Optional, CallbackHandler] function callback);
[RaisesException] void set([NSDictionary=StopAtTopLevel] any items, [Optional, CallbackHandler] function callback);
[RaisesException] void remove([NSObject] any keys, [Optional, CallbackHandler] function callback);
[NeedsPage] void clear([Optional, CallbackHandler] function callback);

Expand Down
47 changes: 42 additions & 5 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIStorage.mm
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
@"browser.test.assertThrows(() => browser?.storage?.local?.set(), /A required argument is missing/i)",
@"browser.test.assertThrows(() => browser?.storage?.local?.set([]), /'items' value is invalid, because an object is expected/i)",
@"browser.test.assertThrows(() => browser?.storage?.local?.set({ 'key': () => { 'function' } }), /it is not JSON-serializable/i)",
@"browser.test.assertThrows(() => browser?.storage?.local?.set({ 'key': 'a'.repeat(8193) }), /exceeded maximum size for a single item/i)",

@"browser.test.assertThrows(() => browser?.storage?.local?.remove(), /A required argument is missing/i)",
@"browser.test.assertThrows(() => browser?.storage?.local?.remove({}), /'keys' value is invalid, because a string or an array of strings is expected, but an object was provided/i)",
Expand Down Expand Up @@ -192,7 +193,7 @@
TEST(WKWebExtensionAPIStorage, Set)
{
auto *backgroundScript = Util::constructScript(@[
@"const data = { 'string': 'string', 'number': 1, 'boolean': true, 'dictionary': {'key': 'value'}, 'array': [1, true, 'string'] }",
@"const data = { 'string': 'string', 'number': 1, 'boolean': true, 'dictionary': {'key': 'value'}, 'array': [1, true, 'string'], 'null': null }",
@"await browser?.storage?.local?.set(data)",

@"var result = await browser?.storage?.local?.get()",
Expand Down Expand Up @@ -220,10 +221,47 @@
Util::loadAndRunExtension(storageManifest, @{ @"background.js": backgroundScript });
}

TEST(WKWebExtensionAPIStorage, SetCustomObject)
{
auto *backgroundScript = Util::constructScript(@[
@"class BaseObject {",
@" constructor(name) {",
@" this.name = name",
@" }",

@" toJSON() {",
@" return { type: 'BaseObject', name: this.name }",
@" }",
@"}",

@"class DerivedObject extends BaseObject {",
@" constructor(name, value) {",
@" super(name)",
@" this.value = value",
@" }",

@" toJSON() {",
@" let baseSerialization = super.toJSON()",
@" return { ...baseSerialization, type: 'DerivedObject', value: this.value }",
@" }",
@"}",

@"const customObject = new DerivedObject('TestObject', 42)",
@"await browser.test.assertSafeResolve(() => browser?.storage?.local?.set({ customObject }))",

@"var result = await browser.test.assertSafeResolve(() => browser?.storage?.local?.get('customObject'))",
@"browser.test.assertDeepEq(customObject?.toJSON(), result?.customObject, 'Custom object should be correctly stored and retrieved')",

@"browser.test.notifyPass()",
]);

Util::loadAndRunExtension(storageManifest, @{ @"background.js": backgroundScript });
}

TEST(WKWebExtensionAPIStorage, Get)
{
auto *backgroundScript = Util::constructScript(@[
@"const data = { 'string': 'string', 'number': 1, 'boolean': true, 'dictionary': {'key': 'value'}, 'array': [1, true, 'string'] }",
@"const data = { 'string': 'string', 'number': 1, 'boolean': true, 'dictionary': {'key': 'value'}, 'array': [1, true, 'string'], 'null': null }",
@"await browser?.storage?.local?.set(data)",

@"var result = await browser?.storage?.local?.get()",
Expand All @@ -235,9 +273,8 @@
@"result = await browser?.storage?.local?.get([ 'string', 'number' ])",
@"browser.test.assertDeepEq({ 'string': 'string', 'number': 1 }, result)",

@"result = await browser?.storage?.local?.get({ 'boolean': false, 'unrecognized_key': 'default_value', 'array': [1, true, 'string'] })",
@"await browser.test.log(result)",
@"browser.test.assertDeepEq({ 'boolean': true, 'unrecognized_key': 'default_value', 'array': [1, true, 'string'] }, result)",
@"result = await browser?.storage?.local?.get({ 'boolean': false, 'unrecognized_key': 'default_value', 'array': [1, true, 'string'], 'null': 42 })",
@"browser.test.assertDeepEq({ 'boolean': true, 'unrecognized_key': 'default_value', 'array': [1, true, 'string'], 'null': null }, result)",

@"browser.test.notifyPass()",
]);
Expand Down

0 comments on commit 944eaf1

Please sign in to comment.