Skip to content

Form submission metadata should be limited to arrays, dictionaries, strings, data, and numbers #11399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
webkit-commit-queue merged 1 commit into from
Mar 18, 2023
Merged

Form submission metadata should be limited to arrays, dictionaries, strings, data, and numbers #11399

webkit-commit-queue merged 1 commit into from
Mar 18, 2023

Conversation

@achristensen07
Copy link
Contributor

@achristensen07 achristensen07 commented Mar 11, 2023 •
edited by webkit-early-warning-system
Loading

20a50d5

Form submission metadata should be limited to arrays, dictionaries, strings, data, and numbers
https://bugs.webkit.org/show_bug.cgi?id=253753
rdar://106585542

Reviewed by Chris Dumez.

Instead of encoding any object using NSSecureCoding then decoding it and returning it to the API client,
restrict the NSObject types to NSString, NSData, NSNumber, NSArray, and NSDictionary, which we convert
to API::Object for serialization, then convert back to those types on the other side.

Also make FormClient.m_controller and FormClient.m_webView WeakObjCPtr smart pointers instead of raw pointers.

Also deprecate _WKProcessPoolConfiguration.customClassesForParameterCoder because it doesn't do anything any more.

* Source/WebKit/Shared/API/APIObject.h:
* Source/WebKit/Shared/Cocoa/APIObject.mm:
(API::Object::toNSObject):
(API::Object::fromNSObject):
* Source/WebKit/UIProcess/API/APIProcessPoolConfiguration.cpp:
(API::ProcessPoolConfiguration::copy):
* Source/WebKit/UIProcess/API/APIProcessPoolConfiguration.h:
* Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.cpp:
(WKContextConfigurationCopyCustomClassesForParameterCoder):
(WKContextConfigurationSetCustomClassesForParameterCoder):
* Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.h:
* Source/WebKit/UIProcess/API/Cocoa/WKProcessGroup.mm:
(-[WKProcessGroup initWithInjectedBundleURL:andCustomClassesForParameterCoder:]):
(toStringVector): Deleted.
* Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _setInputDelegate:]):
* Source/WebKit/UIProcess/API/Cocoa/_WKProcessPoolConfiguration.h:
* Source/WebKit/UIProcess/API/Cocoa/_WKProcessPoolConfiguration.mm:
(-[_WKProcessPoolConfiguration customClassesForParameterCoder]):
(-[_WKProcessPoolConfiguration setCustomClassesForParameterCoder:]):
* Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:
(WebKit::WebProcessPool::platformInitialize):
(WebKit::WebProcessPool::initializeClassesForParameterCoding): Deleted.
(WebKit::WebProcessPool::allowedClassesForParameterCoding const): Deleted.
* Source/WebKit/UIProcess/WebProcessPool.h:
* Source/WebKit/UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::elementDidFocus):
* Source/WebKit/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:
(-[WKWebProcessPlugInBrowserContextController _setFormDelegate:]):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleFormDelegate.mm:
(TEST):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleFormDelegatePlugIn.mm:
(-[BundleFormDelegatePlugInWithUserInfo webProcessPlugIn:didCreateBrowserContextController:]):
(-[BundleFormDelegatePlugInWithUserInfo _webProcessPlugInBrowserContextController:willSubmitForm:toFrame:fromFrame:withValues:]):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKProcessPoolConfiguration.mm:
(TEST):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKInputDelegate.mm:
(-[InputDelegate _webView:willSubmitFormValues:userObject:submissionHandler:]):
(TEST):
* Tools/TestWebKitAPI/WKWebViewConfigurationExtras.h:
* Tools/TestWebKitAPI/WKWebViewConfigurationExtras.mm:
(+[WKWebViewConfiguration _test_configurationWithTestPlugInClassName:configureJSCForTesting:]):
(+[WKWebViewConfiguration _test_configurationWithTestPlugInClassName:configureJSCForTesting:andCustomParameterClasses:]): Deleted.

Canonical link: https://commits.webkit.org/261820@main

1779a28

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim
✅ 🛠 🧪 unsafe-merge

@achristensen07 achristensen07 requested a review from cdumez as a code owner March 11, 2023 04:57
@achristensen07 achristensen07 self-assigned this Mar 11, 2023
@achristensen07 achristensen07 added the WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). label Mar 11, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Mar 11, 2023 •
edited
Loading

EWS run on previous version of this PR (hash c1ea895)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style loading 🛠 ios loading 🛠 mac ❌ 🛠 wpe ⏳ 🛠 wincairo
✅ 🧪 bindings loading 🛠 ios-sim loading 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
✅ 🧪 webkitperl loading 🧪 ios-wk2 loading 🧪 api-mac ❌ 🛠 gtk
loading 🧪 api-ios loading 🧪 mac-wk1 ❌ 🧪 gtk-wk2
⏳ 🛠 🧪 jsc ⏳ 🛠 tv loading 🧪 mac-wk2 ❌ 🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64 loading 🛠 tv-sim loading 🧪 mac-AS-debug-wk2
loading 🛠 watch loading 🧪 mac-wk2-stress
⏳ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 11, 2023
@achristensen07 achristensen07 force-pushed the eng/Form-submission-metadata-should-be-limited-to-arrays-dictionaries-strings-and-numbers branch from c1ea895 to 9042f2f Compare March 11, 2023 06:24
@achristensen07 achristensen07 requested a review from a team as a code owner March 11, 2023 06:24
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Mar 11, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 11, 2023
@achristensen07 achristensen07 force-pushed the eng/Form-submission-metadata-should-be-limited-to-arrays-dictionaries-strings-and-numbers branch from 9042f2f to e3a8776 Compare March 11, 2023 15:37
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Mar 11, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 11, 2023
@achristensen07 achristensen07 force-pushed the eng/Form-submission-metadata-should-be-limited-to-arrays-dictionaries-strings-and-numbers branch 2 times, most recently from 8ecd9d1 to 43d2f6b Compare March 11, 2023 16:02
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Mar 11, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Mar 12, 2023 •
edited
Loading

EWS run on previous version of this PR (hash 43d2f6b)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl   🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 webkitpy   🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
⏳ 🛠 🧪 jsc ✅ 🛠 tv ✅ 🧪 mac-wk2   🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64 ✅ 🛠 tv-sim   🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Mar 12, 2023

EWS run on previous version of this PR (hash 8ecd9d1)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
⏳ 🧪 style ⏳ 🛠 ios ⏳ 🛠 mac ❌ 🛠 wpe ✅ 🛠 wincairo
⏳ 🧪 bindings ⏳ 🛠 ios-sim ⏳ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
⏳ 🧪 webkitperl ⏳ 🧪 ios-wk2 ⏳ 🧪 api-mac ❌ 🛠 gtk
⏳ 🧪 webkitpy ⏳ 🧪 api-ios ⏳ 🧪 mac-wk1 ❌ 🧪 gtk-wk2
⏳ 🛠 🧪 jsc ⏳ 🛠 tv ⏳ 🧪 mac-wk2 ❌ 🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64 ⏳ 🛠 tv-sim ⏳ 🧪 mac-AS-debug-wk2
⏳ 🧪 services ⏳ 🛠 watch ⏳ 🧪 mac-wk2-stress
⏳ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Mar 12, 2023

EWS run on previous version of this PR (hash e3a8776)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
⏳ 🧪 style ⏳ 🛠 ios ⏳ 🛠 mac ❌ 🛠 wpe ✅ 🛠 wincairo
⏳ 🧪 bindings ⏳ 🛠 ios-sim ⏳ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
⏳ 🧪 webkitperl ⏳ 🧪 ios-wk2 ⏳ 🧪 api-mac   🛠 gtk
⏳ 🧪 webkitpy ⏳ 🧪 api-ios ⏳ 🧪 mac-wk1   🧪 gtk-wk2
⏳ 🛠 🧪 jsc ⏳ 🛠 tv ⏳ 🧪 mac-wk2   🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64 ⏳ 🛠 tv-sim ⏳ 🧪 mac-AS-debug-wk2
⏳ 🧪 services ⏳ 🛠 watch ⏳ 🧪 mac-wk2-stress
⏳ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Mar 12, 2023

EWS run on previous version of this PR (hash 9042f2f)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
⏳ 🧪 style ⏳ 🛠 ios ⏳ 🛠 mac ❌ 🛠 wpe ✅ 🛠 wincairo
⏳ 🧪 bindings ⏳ 🛠 ios-sim ⏳ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
⏳ 🧪 webkitperl ⏳ 🧪 ios-wk2 ⏳ 🧪 api-mac ❌ 🛠 gtk
⏳ 🧪 webkitpy ⏳ 🧪 api-ios ⏳ 🧪 mac-wk1 ❌ 🧪 gtk-wk2
⏳ 🛠 🧪 jsc ⏳ 🛠 tv ⏳ 🧪 mac-wk2 ❌ 🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64 ⏳ 🛠 tv-sim ⏳ 🧪 mac-AS-debug-wk2
⏳ 🧪 services ⏳ 🛠 watch ⏳ 🧪 mac-wk2-stress
⏳ 🛠 watch-sim

@achristensen07 achristensen07 force-pushed the eng/Form-submission-metadata-should-be-limited-to-arrays-dictionaries-strings-and-numbers branch from 43d2f6b to 08ec9f3 Compare March 12, 2023 06:50
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Mar 12, 2023 •
edited
Loading

EWS run on previous version of this PR (hash 08ec9f3)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ⏳ 🛠 mac-AS-debug   🧪 wpe-wk2
✅ 🧪 webkitperl ⏳ 🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
✅ 🧪 webkitpy ⏳ 🧪 api-ios   🧪 mac-wk1   🧪 gtk-wk2
⏳ 🛠 🧪 jsc ✅ 🛠 tv   🧪 mac-wk2 ⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64 ✅ 🛠 tv-sim ⏳ 🧪 mac-AS-debug-wk2
✅ 🛠 watch   🧪 mac-wk2-stress
✅ 🛠 watch-sim

@achristensen07 achristensen07 force-pushed the eng/Form-submission-metadata-should-be-limited-to-arrays-dictionaries-strings-and-numbers branch from 08ec9f3 to edb3062 Compare March 12, 2023 07:02
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Mar 12, 2023 •
edited
Loading

EWS run on previous version of this PR (hash edb3062)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 webkitpy ✅ 🧪 api-ios ⏳ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-wk2   🧪 api-gtk
✅ 🛠 tv-sim   🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim

@achristensen07 achristensen07 changed the title Form submission metadata should be limited to arrays, dictionaries, strings, and numbers Form submission metadata should be limited to arrays, dictionaries, strings, data, and numbers Mar 13, 2023
@achristensen07 achristensen07 force-pushed the eng/Form-submission-metadata-should-be-limited-to-arrays-dictionaries-strings-and-numbers branch from edb3062 to 1779a28 Compare March 13, 2023 21:08
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Mar 13, 2023 •
edited
Loading

EWS run on current version of this PR (hash 1779a28)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim
✅ 🛠 🧪 unsafe-merge

cdumez
cdumez approved these changes Mar 13, 2023
View reviewed changes
Copy link
Contributor

@cdumez cdumez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if the bots are happy.

@achristensen07 achristensen07 added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 18, 2023
@achristensen07
Form submission metadata should be limited to arrays, dictionaries, s…
20a50d5 
…trings, data, and numbers

https://bugs.webkit.org/show_bug.cgi?id=253753
rdar://106585542

Reviewed by Chris Dumez.

Instead of encoding any object using NSSecureCoding then decoding it and returning it to the API client,
restrict the NSObject types to NSString, NSData, NSNumber, NSArray, and NSDictionary, which we convert
to API::Object for serialization, then convert back to those types on the other side.

Also make FormClient.m_controller and FormClient.m_webView WeakObjCPtr smart pointers instead of raw pointers.

Also deprecate _WKProcessPoolConfiguration.customClassesForParameterCoder because it doesn't do anything any more.

* Source/WebKit/Shared/API/APIObject.h:
* Source/WebKit/Shared/Cocoa/APIObject.mm:
(API::Object::toNSObject):
(API::Object::fromNSObject):
* Source/WebKit/UIProcess/API/APIProcessPoolConfiguration.cpp:
(API::ProcessPoolConfiguration::copy):
* Source/WebKit/UIProcess/API/APIProcessPoolConfiguration.h:
* Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.cpp:
(WKContextConfigurationCopyCustomClassesForParameterCoder):
(WKContextConfigurationSetCustomClassesForParameterCoder):
* Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.h:
* Source/WebKit/UIProcess/API/Cocoa/WKProcessGroup.mm:
(-[WKProcessGroup initWithInjectedBundleURL:andCustomClassesForParameterCoder:]):
(toStringVector): Deleted.
* Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _setInputDelegate:]):
* Source/WebKit/UIProcess/API/Cocoa/_WKProcessPoolConfiguration.h:
* Source/WebKit/UIProcess/API/Cocoa/_WKProcessPoolConfiguration.mm:
(-[_WKProcessPoolConfiguration customClassesForParameterCoder]):
(-[_WKProcessPoolConfiguration setCustomClassesForParameterCoder:]):
* Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:
(WebKit::WebProcessPool::platformInitialize):
(WebKit::WebProcessPool::initializeClassesForParameterCoding): Deleted.
(WebKit::WebProcessPool::allowedClassesForParameterCoding const): Deleted.
* Source/WebKit/UIProcess/WebProcessPool.h:
* Source/WebKit/UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::elementDidFocus):
* Source/WebKit/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:
(-[WKWebProcessPlugInBrowserContextController _setFormDelegate:]):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleFormDelegate.mm:
(TEST):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleFormDelegatePlugIn.mm:
(-[BundleFormDelegatePlugInWithUserInfo webProcessPlugIn:didCreateBrowserContextController:]):
(-[BundleFormDelegatePlugInWithUserInfo _webProcessPlugInBrowserContextController:willSubmitForm:toFrame:fromFrame:withValues:]):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKProcessPoolConfiguration.mm:
(TEST):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKInputDelegate.mm:
(-[InputDelegate _webView:willSubmitFormValues:userObject:submissionHandler:]):
(TEST):
* Tools/TestWebKitAPI/WKWebViewConfigurationExtras.h:
* Tools/TestWebKitAPI/WKWebViewConfigurationExtras.mm:
(+[WKWebViewConfiguration _test_configurationWithTestPlugInClassName:configureJSCForTesting:]):
(+[WKWebViewConfiguration _test_configurationWithTestPlugInClassName:configureJSCForTesting:andCustomParameterClasses:]): Deleted.

Canonical link: https://commits.webkit.org/261820@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Form-submission-metadata-should-be-limited-to-arrays-dictionaries-strings-and-numbers branch from 1779a28 to 20a50d5 Compare March 18, 2023 03:43
@webkit-commit-queue
Copy link
Collaborator

webkit-commit-queue commented Mar 18, 2023

Committed 261820@main (20a50d5): https://commits.webkit.org/261820@main

Reviewed commits have been landed. Closing PR #11399 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 18, 2023
@webkit-commit-queue webkit-commit-queue merged commit 20a50d5 into WebKit:main Mar 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cdumez cdumez cdumez approved these changes

Assignees

@achristensen07 achristensen07

Labels

WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@achristensen07 @webkit-early-warning-system @webkit-commit-queue @cdumez @webkit-ews-buildbot