Skip to content

Commit

Permalink
Add stricter type checking to the Web Extension code generator.
Browse files Browse the repository at this point in the history
https://webkit.org/b/261386
rdar://problem/115255883

Reviewed by Brian Weinstein.

* Have the code generator do strict type checks per argument now that the argument handling
  is simplified and easier to add those checks. This aligns with Chrome and Firefox type checks.
* Simplified validateDictionary to not take optionalKeys, this reduces the number of static
  NSSets we have for each API. Now only the dictionary of types is required for most APIs.
* Tweak the wording to not say "string value" and "number value", to be just "string" and "number".
* Have _WKWebExtensionWebNavigationURLFilter use the new toErrorString and validateObject functions,
  which was missed in my earlier error message refactoring.

* Source/WebKit/Shared/Extensions/WebExtensionUtilities.h:
* Source/WebKit/Shared/Extensions/WebExtensionUtilities.mm:
(WebKit::classToClassString): Changed "string value" and "number value", to be just "string" and "number".
(WebKit::validateDictionary): Dropped optionalKeys, and generate it from the types and required set.
(WebKit::validateObject): Added an ASSERT.
* Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIAlarmsCocoa.mm:
(WebKit::WebExtensionAPIAlarms::createAlarm): Removed optionalKeys.
* Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIPermissionsCocoa.mm:
(WebKit::WebExtensionAPIPermissions::parseDetailsDictionary): Removed optionalKeys.
* Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPITabsCocoa.mm:
(WebKit::WebExtensionAPITabs::parseTabCreateOptions): Removed optionalKeys.
(WebKit::WebExtensionAPITabs::parseTabUpdateOptions): Removed optionalKeys.
(WebKit::WebExtensionAPITabs::parseTabQueryOptions): Removed optionalKeys.
(WebKit::WebExtensionAPITabs::reload): Removed optionalKeys.
* Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIWindowsCocoa.mm:
(WebKit::WebExtensionAPIWindows::parsePopulateTabs): Removed optionalKeys.
(WebKit::WebExtensionAPIWindows::parseWindowTypesFilter): Removed optionalKeys.
(WebKit::WebExtensionAPIWindows::parseWindowCreateOptions): Removed optionalKeys.
(WebKit::WebExtensionAPIWindows::parseWindowUpdateOptions): Removed optionalKeys.
* Source/WebKit/WebProcess/Extensions/Bindings/Scripts/CodeGeneratorExtensions.pm:
(_generateImplementationFile): Added calls to _installArgumentTypeExceptions for arguments.
(_installArgumentTypeExceptions): Added.
(_javaScriptTypeCondition): Only allow null and undefined when the argument is optional. This matches Chrome and Firefox.
(_platformTypeVariableDeclaration): Drop condition for numbers since _installArgumentTypeExceptions handles it now.
* Source/WebKit/WebProcess/Extensions/Cocoa/_WKWebExtensionWebNavigationURLFilter.mm:
(-[_WKWebExtensionWebNavigationURLPredicate initWithTypeString:value:outErrorMessage:]): Use validateObject and toErrorString.
(-[_WKWebExtensionWebNavigationURLFilter initWithDictionary:outErrorMessage:]): Drop urlOptionalKeys.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIAlarms.mm:
(TestWebKitAPI::TEST): Updated error string expectations.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIPermissions.mm:
(TestWebKitAPI::TEST): Updated error string expectations.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPITabs.mm:
(TestWebKitAPI::TEST): Updated error string expectations. Added back some error checks that work again with generator changes.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIWebNavigation.mm:
(TestWebKitAPI::TEST): Updated error string expectations.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIWindows.mm:
(TestWebKitAPI::TEST): Updated error string expectations.

Canonical link: https://commits.webkit.org/267883@main
  • Loading branch information
xeenon committed Sep 11, 2023
1 parent 1d4746c commit bc22aaa
Show file tree
Hide file tree
Showing 13 changed files with 237 additions and 222 deletions.
5 changes: 2 additions & 3 deletions Source/WebKit/Shared/Extensions/WebExtensionUtilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,15 @@
namespace WebKit {

/// Verifies that a dictionary:
/// - Contains a required set of string keys, as listed in `requiredKeys`.
/// - Has no unexpected keys beyond the required keys and an additional set of optional keys, listed in `optionalKeys`.
/// - Contains a required set of string keys, as listed in `requiredKeys`, all other types specified in `keyTypes` are optional keys.
/// - Has values that are the appropriate type for each key, as specified by `keyTypes`. The keys in this dictionary
/// correspond to keys in the original dictionary being validated, and the values in `keyTypes` may be:
/// - A `Class`, that the value in the original dictionary must be a kind of.
/// - An `NSArray` containing one class, specifying that the value in the original dictionary must be an array with elements that are a kind of the specified class.
/// - An `NSOrderedSet` containing one or more classes or arrays, specifying the value in the dictionary should be of any classes in the set or their subclasses.
/// - The `callingAPIName` and `sourceKey` parameters are used to reference the object within a larger context. When an error occurs, this key helps identify the source of the problem in the `outExceptionString`.
/// If the dictionary is valid, returns `YES`. Otherwise returns `NO` and sets `outExceptionString` to a message describing what validation failed.
bool validateDictionary(NSDictionary<NSString *, id> *, NSString *sourceKey, NSArray<NSString *> *requiredKeys, NSArray<NSString *> *optionalKeys, NSDictionary<NSString *, id> *keyTypes, NSString **outExceptionString);
bool validateDictionary(NSDictionary<NSString *, id> *, NSString *sourceKey, NSArray<NSString *> *requiredKeys, NSDictionary<NSString *, id> *keyTypes, NSString **outExceptionString);

/// Verifies a single object against a certain type criteria:
/// - Checks that the object matches the type defined in `valueTypes`. The `valueTypes` can be:
Expand Down
22 changes: 14 additions & 8 deletions Source/WebKit/Shared/Extensions/WebExtensionUtilities.mm
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,17 @@
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
classTypeToSingularClassString = [NSMapTable strongToStrongObjectsMapTable];
[classTypeToSingularClassString setObject:@"a boolean value" forKey:@YES.class];
[classTypeToSingularClassString setObject:@"a number value" forKey:NSNumber.class];
[classTypeToSingularClassString setObject:@"a string value" forKey:NSString.class];
[classTypeToSingularClassString setObject:@"a boolean" forKey:@YES.class];
[classTypeToSingularClassString setObject:@"a number" forKey:NSNumber.class];
[classTypeToSingularClassString setObject:@"a string" forKey:NSString.class];
[classTypeToSingularClassString setObject:@"null" forKey:NSNull.class];
[classTypeToSingularClassString setObject:@"an array" forKey:NSArray.class];
[classTypeToSingularClassString setObject:@"an object" forKey:NSDictionary.class];

classTypeToPluralClassString = [NSMapTable strongToStrongObjectsMapTable];
[classTypeToPluralClassString setObject:@"boolean values" forKey:@YES.class];
[classTypeToPluralClassString setObject:@"number values" forKey:NSNumber.class];
[classTypeToPluralClassString setObject:@"string values" forKey:NSString.class];
[classTypeToPluralClassString setObject:@"booleans" forKey:@YES.class];
[classTypeToPluralClassString setObject:@"numbers" forKey:NSNumber.class];
[classTypeToPluralClassString setObject:@"strings" forKey:NSString.class];
[classTypeToPluralClassString setObject:@"null values" forKey:NSNull.class];
[classTypeToPluralClassString setObject:@"arrays" forKey:NSArray.class];
[classTypeToPluralClassString setObject:@"objects" forKey:NSDictionary.class];
Expand Down Expand Up @@ -223,11 +223,15 @@ static bool validate(NSString *key, NSObject *value, id expectedValueType, NSStr
return [NSString stringWithFormat:@"'%@', and '%@'", formattedInitialItems, list.lastObject];
}

bool validateDictionary(NSDictionary<NSString *, id> *dictionary, NSString *sourceKey, NSArray<NSString *> *requiredKeys, NSArray<NSString *> *optionalKeys, NSDictionary<NSString *, id> *keyTypes, NSString **outExceptionString)
bool validateDictionary(NSDictionary<NSString *, id> *dictionary, NSString *sourceKey, NSArray<NSString *> *requiredKeys, NSDictionary<NSString *, id> *keyTypes, NSString **outExceptionString)
{
ASSERT(keyTypes.count);

NSMutableSet<NSString *> *remainingRequiredKeys = [[NSMutableSet alloc] initWithArray:requiredKeys];
NSSet<NSString *> *requiredKeysSet = [[NSSet alloc] initWithArray:requiredKeys];
NSSet<NSString *> *optionalKeysSet = [[NSSet alloc] initWithArray:optionalKeys];

NSMutableSet<NSString *> *optionalKeysSet = [[NSMutableSet alloc] initWithArray:keyTypes.allKeys];
[optionalKeysSet minusSet:requiredKeysSet];

__block NSString *errorString;

Expand Down Expand Up @@ -262,6 +266,8 @@ bool validateDictionary(NSDictionary<NSString *, id> *dictionary, NSString *sour

bool validateObject(NSObject *object, NSString *sourceKey, id expectedValueType, NSString **outExceptionString)
{
ASSERT(expectedValueType);

NSString *errorString;
validate(nil, object, expectedValueType, &errorString);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,19 +85,13 @@
{
// Documentation: https://developer.mozilla.org/docs/Mozilla/Add-ons/WebExtensions/API/alarms/create

static NSArray<NSString *> *optionalKeys = @[
whenKey,
delayInMinutesKey,
periodInMinutesKey,
];

static NSDictionary<NSString *, id> *types = @{
whenKey: NSNumber.class,
delayInMinutesKey: NSNumber.class,
periodInMinutesKey: NSNumber.class,
};

if (!validateDictionary(alarmInfo, @"info", nil, optionalKeys, types, outExceptionString))
if (!validateDictionary(alarmInfo, @"info", nil, types, outExceptionString))
return;

auto *whenNumber = objectForKey<NSNumber>(alarmInfo, whenKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,17 +146,12 @@

bool WebExtensionAPIPermissions::parseDetailsDictionary(NSDictionary *details, HashSet<String>& permissions, HashSet<String>& origins, NSString *callingAPIName, NSString **outExceptionString)
{
static NSArray<NSString *> *optionalKeys = @[
permissionsKey,
originsKey,
];

static NSDictionary<NSString *, id> *types = @{
permissionsKey: @[ NSString.class ],
originsKey: @[ NSString.class ],
};

if (!validateDictionary(details, permissionsKey, nil, optionalKeys, types, outExceptionString))
if (!validateDictionary(details, permissionsKey, nil, types, outExceptionString))
return false;

for (NSString *permission in objectForKey<NSArray>(details, permissionsKey, true, NSString.class))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,21 +167,14 @@
if (!parseTabUpdateOptions(options, parameters, sourceKey, outExceptionString))
return false;

static NSArray<NSString *> *optionalKeys = @[
indexKey,
openInReaderModeKey,
titleKey,
windowIdKey,
];

static NSDictionary<NSString *, id> *types = @{
indexKey: NSNumber.class,
openInReaderModeKey: @YES.class,
titleKey: NSString.class,
windowIdKey: NSNumber.class,
};

if (!validateDictionary(options, sourceKey, nil, optionalKeys, types, outExceptionString))
if (!validateDictionary(options, sourceKey, nil, types, outExceptionString))
return false;

if (NSNumber *windowId = objectForKey<NSNumber>(options, windowIdKey)) {
Expand All @@ -207,16 +200,6 @@

bool WebExtensionAPITabs::parseTabUpdateOptions(NSDictionary *options, WebExtensionTabParameters& parameters, NSString *sourceKey, NSString **outExceptionString)
{
static NSArray<NSString *> *optionalKeys = @[
activeKey,
highlightedKey,
mutedKey,
openerTabIdKey,
pinnedKey,
selectedKey,
urlKey,
];

static NSDictionary<NSString *, id> *types = @{
activeKey: @YES.class,
highlightedKey: @YES.class,
Expand All @@ -227,7 +210,7 @@
urlKey: NSString.class,
};

if (!validateDictionary(options, sourceKey, nil, optionalKeys, types, outExceptionString))
if (!validateDictionary(options, sourceKey, nil, types, outExceptionString))
return false;

if (NSString *url = objectForKey<NSString>(options, urlKey)) {
Expand Down Expand Up @@ -268,24 +251,6 @@

bool WebExtensionAPITabs::parseTabQueryOptions(NSDictionary *options, WebExtensionTabQueryParameters& parameters, NSString *sourceKey, NSString **outExceptionString)
{
static NSArray<NSString *> *optionalKeys = @[
activeKey,
audibleKey,
currentWindowKey,
hiddenKey,
highlightedKey,
indexKey,
lastFocusedWindowKey,
mutedKey,
pinnedKey,
selectedKey,
statusKey,
titleKey,
urlKey,
windowIdKey,
windowTypeKey,
];

static NSDictionary<NSString *, id> *types = @{
activeKey: @YES.class,
audibleKey: @YES.class,
Expand All @@ -304,7 +269,7 @@
windowTypeKey: NSString.class,
};

if (!validateDictionary(options, sourceKey, nil, optionalKeys, types, outExceptionString))
if (!validateDictionary(options, sourceKey, nil, types, outExceptionString))
return false;

if (NSNumber *windowId = objectForKey<NSNumber>(options, windowIdKey)) {
Expand Down Expand Up @@ -595,15 +560,11 @@ static bool isValid(std::optional<WebExtensionTabIdentifier> identifier, NSStrin
if (tabIdentifer && !isValid(tabIdentifer, outExceptionString))
return;

static NSArray<NSString *> *optionalKeys = @[
bypassCacheKey,
];

static NSDictionary<NSString *, id> *types = @{
bypassCacheKey: @YES.class,
};

if (!validateDictionary(options, @"properties", nil, optionalKeys, types, outExceptionString))
if (!validateDictionary(options, @"properties", nil, types, outExceptionString))
return;

using ReloadFromOrigin = WebExtensionContext::ReloadFromOrigin;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,15 +181,11 @@

bool WebExtensionAPIWindows::parsePopulateTabs(NSDictionary *options, PopulateTabs& populate, NSString *sourceKey, NSString **outExceptionString)
{
static NSArray<NSString *> *optionalKeys = @[
populateKey,
];

static NSDictionary<NSString *, id> *types = @{
populateKey: @YES.class,
};

if (!validateDictionary(options, sourceKey, nil, optionalKeys, types, outExceptionString))
if (!validateDictionary(options, sourceKey, nil, types, outExceptionString))
return false;

populate = objectForKey<NSNumber>(options, populateKey).boolValue ? PopulateTabs::Yes : PopulateTabs::No;
Expand All @@ -202,15 +198,11 @@
// All windows match by default.
windowTypeFilter = WindowTypeFilter::All;

static NSArray<NSString *> *optionalKeys = @[
windowTypesKey,
];

static NSDictionary<NSString *, id> *types = @{
windowTypesKey: @[ NSString.class ],
};

if (!validateDictionary(options, sourceKey, nil, optionalKeys, types, outExceptionString))
if (!validateDictionary(options, sourceKey, nil, types, outExceptionString))
return false;

NSArray<NSString *> *windowTypes = objectForKey<NSArray>(options, windowTypesKey, false, NSString.class);
Expand Down Expand Up @@ -262,21 +254,14 @@
if (!parseWindowUpdateOptions(options, parameters, sourceKey, outExceptionString))
return false;

static NSArray<NSString *> *optionalKeys = @[
typeKey,
incognitoKey,
urlKey,
tabIdKey,
];

static NSDictionary<NSString *, id> *types = @{
typeKey: NSString.class,
incognitoKey: @YES.class,
urlKey: [NSOrderedSet orderedSetWithObjects:NSString.class, @[ NSString.class ], nil],
tabIdKey: NSNumber.class,
};

if (!validateDictionary(options, sourceKey, nil, optionalKeys, types, outExceptionString))
if (!validateDictionary(options, sourceKey, nil, types, outExceptionString))
return false;

if (NSString *type = objectForKey<NSString>(options, typeKey))
Expand Down Expand Up @@ -328,15 +313,6 @@

bool WebExtensionAPIWindows::parseWindowUpdateOptions(NSDictionary *options, WebExtensionWindowParameters& parameters, NSString *sourceKey, NSString **outExceptionString)
{
static NSArray<NSString *> *optionalKeys = @[
stateKey,
focusedKey,
topKey,
leftKey,
widthKey,
heightKey,
];

static NSDictionary<NSString *, id> *types = @{
stateKey: NSString.class,
focusedKey: @YES.class,
Expand All @@ -346,7 +322,7 @@
heightKey: NSNumber.class,
};

if (!validateDictionary(options, sourceKey, nil, optionalKeys, types, outExceptionString))
if (!validateDictionary(options, sourceKey, nil, types, outExceptionString))
return false;

if (NSString *state = objectForKey<NSString>(options, stateKey, true))
Expand Down
Loading

0 comments on commit bc22aaa

Please sign in to comment.