Skip to content
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.

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

Check the correct network connection integrity policy after a navigation #9955

Conversation

charliewolfe
Copy link
Member

@charliewolfe charliewolfe commented Feb 11, 2023

801abd6

Check the correct network connection integrity policy after a navigation
https://bugs.webkit.org/show_bug.cgi?id=252089
rdar://104452273

Reviewed by Alex Christensen.

In some cases where we check the network connection integrity policy, we are checking the
policy in the context of a navigation that may have occurred. This can cause issues when the
network connection integrity policy was set, but changed during navigation. In this situation,
the network connection integrity policy we are interested in checking is the one set on the page
we navigated away from. To fix this, we preserve the policy set on the old document loader to
the new one so we can check it on the destination site.

* Source/WebCore/dom/Document.cpp:
(WebCore::Document::urlForBindings const):
(WebCore::Document::referrerForBindings):
* Source/WebCore/loader/DocumentLoader.h:
(WebCore::DocumentLoader::setOriginalNetworkConnectionIntegrityPolicy):
(WebCore::DocumentLoader::originalNetworkConnectionIntegrityPolicy const):
* Source/WebCore/loader/FrameLoadRequest.h:
(WebCore::FrameLoadRequest::setNetworkConnectionIntegrityPolicy):
(WebCore::FrameLoadRequest::networkConnectionIntegrityPolicy const):
* Source/WebCore/loader/FrameLoader.cpp:
(WebCore::FrameLoader::load):
* Source/WebKit/Shared/LoadParameters.cpp:
(WebKit::LoadParameters::encode const):
(WebKit::LoadParameters::decode):
* Source/WebKit/Shared/LoadParameters.h:
* Source/WebKit/Shared/NavigationActionData.h:
* Source/WebKit/Shared/NavigationActionData.serialization.in:
* Source/WebKit/UIProcess/API/APINavigation.h:
(API::Navigation::setOriginatorNetworkConnectionIntegrityPolicy):
(API::Navigation::originatorNetworkConnectionIntegrityPolicy const):
* Source/WebKit/Shared/WebsitePoliciesData.cpp:
(WebKit::WebsitePoliciesData::applyToDocumentLoader):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::loadRequestWithNavigationShared):
(WebKit::WebPageProxy::receivedNavigationPolicyDecision):
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::createWindow):
* Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::didSameDocumentNavigationForFrameViaJSHistoryAPI):
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction):
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
* Source/WebKit/WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::info const):
(WebKit::WebFrame::networkConnectionIntegrityPolicy const):
* Source/WebKit/WebProcess/WebPage/WebFrame.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::loadRequest):

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

15fd7ee

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
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 ❌ πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress
  πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@charliewolfe charliewolfe self-assigned this Feb 11, 2023
@charliewolfe charliewolfe added the WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). label Feb 11, 2023
@charliewolfe charliewolfe marked this pull request as draft February 11, 2023 00:22
@charliewolfe charliewolfe force-pushed the eng/Check-the-correct-network-connection-integrity-policy-after-a-navigation branch from 4c3d11f to de465ce Compare February 11, 2023 00:47
@charliewolfe charliewolfe force-pushed the eng/Check-the-correct-network-connection-integrity-policy-after-a-navigation branch from de465ce to 8b642f8 Compare February 11, 2023 02:07
@charliewolfe charliewolfe force-pushed the eng/Check-the-correct-network-connection-integrity-policy-after-a-navigation branch from 8b642f8 to 9b96b9e Compare February 11, 2023 02:13
@whsieh whsieh requested review from achristensen07 and hortont424 and removed request for achristensen07 February 11, 2023 02:27
@charliewolfe charliewolfe force-pushed the eng/Check-the-correct-network-connection-integrity-policy-after-a-navigation branch from 9b96b9e to 4511b19 Compare February 12, 2023 20:28
@charliewolfe charliewolfe force-pushed the eng/Check-the-correct-network-connection-integrity-policy-after-a-navigation branch from 4511b19 to 5035c22 Compare February 13, 2023 21:17
@charliewolfe charliewolfe force-pushed the eng/Check-the-correct-network-connection-integrity-policy-after-a-navigation branch from 5035c22 to 4a74de8 Compare February 14, 2023 20:12
@charliewolfe charliewolfe marked this pull request as ready for review February 14, 2023 20:12
@charliewolfe charliewolfe force-pushed the eng/Check-the-correct-network-connection-integrity-policy-after-a-navigation branch from 4a74de8 to 6bd5e9f Compare April 24, 2023 21:11
@charliewolfe charliewolfe force-pushed the eng/Check-the-correct-network-connection-integrity-policy-after-a-navigation branch from 6bd5e9f to fddba55 Compare April 25, 2023 23:59
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 26, 2023
@charliewolfe charliewolfe removed the merging-blocked Applied to prevent a change from being merged label Apr 26, 2023
@charliewolfe charliewolfe force-pushed the eng/Check-the-correct-network-connection-integrity-policy-after-a-navigation branch from fddba55 to b402390 Compare April 26, 2023 00:50
@@ -1113,4 +1113,11 @@ uint64_t WebFrame::messageSenderDestinationID() const
return m_frameID.object().toUInt64();
}

OptionSet<WebCore::NetworkConnectionIntegrity> WebFrame::networkConnectionIntegrityPolicy() const
{
if (!coreFrame() || !coreFrame()->document() || !coreFrame()->document()->topDocument().loader())
Copy link
Contributor

Choose a reason for hiding this comment

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

auto* coreFrame = this->coreFrame();
if (!coreFrame)
return { };
auto* document = coreFrame->document();
if (!document)
return { };
auto* topDocumentLoader = document->topDocument().loader();
if (!topDocumentLoader)
return { };
return topDocumentLoader->networkConnectionIntegrityPolicy();

That way, we don't have any duplicate function calls.

Also, I'm not sure how this interacts with #13108 where we're no longer always using the top document loader as the source. One of them needs to be updated, I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this function will need to change. I'll land this patch first and change this function in the other patch to check the current frame's document loader whenever the main frame URL has a custom scheme.

@charliewolfe charliewolfe force-pushed the eng/Check-the-correct-network-connection-integrity-policy-after-a-navigation branch from b402390 to 15fd7ee Compare April 26, 2023 01:30
@charliewolfe charliewolfe added merge-queue Applied to send a pull request to merge-queue and removed merge-queue Applied to send a pull request to merge-queue labels Apr 26, 2023
https://bugs.webkit.org/show_bug.cgi?id=252089
rdar://104452273

Reviewed by Alex Christensen.

In some cases where we check the network connection integrity policy, we are checking the
policy in the context of a navigation that may have occurred. This can cause issues when the
network connection integrity policy was set, but changed during navigation. In this situation,
the network connection integrity policy we are interested in checking is the one set on the page
we navigated away from. To fix this, we preserve the policy set on the old document loader to
the new one so we can check it on the destination site.

* Source/WebCore/dom/Document.cpp:
(WebCore::Document::urlForBindings const):
(WebCore::Document::referrerForBindings):
* Source/WebCore/loader/DocumentLoader.h:
(WebCore::DocumentLoader::setOriginalNetworkConnectionIntegrityPolicy):
(WebCore::DocumentLoader::originalNetworkConnectionIntegrityPolicy const):
* Source/WebCore/loader/FrameLoadRequest.h:
(WebCore::FrameLoadRequest::setNetworkConnectionIntegrityPolicy):
(WebCore::FrameLoadRequest::networkConnectionIntegrityPolicy const):
* Source/WebCore/loader/FrameLoader.cpp:
(WebCore::FrameLoader::load):
* Source/WebKit/Shared/LoadParameters.cpp:
(WebKit::LoadParameters::encode const):
(WebKit::LoadParameters::decode):
* Source/WebKit/Shared/LoadParameters.h:
* Source/WebKit/Shared/NavigationActionData.h:
* Source/WebKit/Shared/NavigationActionData.serialization.in:
* Source/WebKit/UIProcess/API/APINavigation.h:
(API::Navigation::setOriginatorNetworkConnectionIntegrityPolicy):
(API::Navigation::originatorNetworkConnectionIntegrityPolicy const):
* Source/WebKit/Shared/WebsitePoliciesData.cpp:
(WebKit::WebsitePoliciesData::applyToDocumentLoader):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::loadRequestWithNavigationShared):
(WebKit::WebPageProxy::receivedNavigationPolicyDecision):
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::createWindow):
* Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::didSameDocumentNavigationForFrameViaJSHistoryAPI):
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction):
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
* Source/WebKit/WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::info const):
(WebKit::WebFrame::networkConnectionIntegrityPolicy const):
* Source/WebKit/WebProcess/WebPage/WebFrame.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::loadRequest):

Canonical link: https://commits.webkit.org/263421@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Check-the-correct-network-connection-integrity-policy-after-a-navigation branch from 15fd7ee to 801abd6 Compare April 26, 2023 18:18
@webkit-commit-queue
Copy link
Collaborator

Committed 263421@main (801abd6): https://commits.webkit.org/263421@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 26, 2023
@webkit-commit-queue webkit-commit-queue merged commit 801abd6 into WebKit:main Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore).
Projects
None yet
6 participants