Make sure secure websocket connections in service workers can trigger authentication challenge callbacks
<rdar://85888177>
Created attachment 448217 [details] Patch
Created attachment 448245 [details] Patch
Created attachment 448250 [details] Patch
Created attachment 448272 [details] Patch
Created attachment 448281 [details] Patch
Comment on attachment 448250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448250&action=review r=me > Source/WebKit/NetworkProcess/cocoa/WebSocketTaskCocoa.h:73 > + const std::optional<WebCore::SecurityOriginData>& topOrigin() const { return m_topOrigin; } SecurityOriginData has an "empty" state so we could use a SecurityOriginData as data members (instead of a std::optional<>) and have the call sites check SecurityOriginData::isEmpty(). > Source/WebKit/NetworkProcess/cocoa/WebSocketTaskCocoa.mm:51 > + if (clientOrigin.topOrigin == clientOrigin.clientOrigin) This could use a comment to explain why we only initialize m_topOrigin if topOrigin is the same as clientOrigin. > Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:121 > + MessageSender::send(Messages::NetworkConnectionToWebProcess::CreateSocketChannel { *request, protocol, m_identifier, m_webPageProxyID, ClientOrigin { m_document->topOrigin().data(), m_document->securityOrigin().data() } }); I bet we have this logic in several places and it may be worth having a getter on Document that returns a ClientOrigin at this point.
Comment on attachment 448250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448250&action=review > LayoutTests/platform/mac-bigsur-wk2/http/tests/workers/service/serviceworker-websocket.https-expected.txt:3 > +PASS Open a WebSocket in service worker These files are identical to the platform independent version and unnecessary.
(In reply to Alex Christensen from comment #8) > Comment on attachment 448250 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=448250&action=review > > > LayoutTests/platform/mac-bigsur-wk2/http/tests/workers/service/serviceworker-websocket.https-expected.txt:3 > > +PASS Open a WebSocket in service worker > > These files are identical to the platform independent version and > unnecessary. Not really, they are missing the canAuthenticateAgainstProtectionSpace line. I guess we could reverse the files but in the future, all cocoa ports will probably be aligned with the updated test expectation.
Comment on attachment 448250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448250&action=review >>> LayoutTests/platform/mac-bigsur-wk2/http/tests/workers/service/serviceworker-websocket.https-expected.txt:3 >>> +PASS Open a WebSocket in service worker >> >> These files are identical to the platform independent version and unnecessary. > > Not really, they are missing the canAuthenticateAgainstProtectionSpace line. > I guess we could reverse the files but in the future, all cocoa ports will probably be aligned with the updated test expectation. oh, of course. I was looking at it wrong. This is fine.
Created attachment 448365 [details] Patch for landing
Committed r287611 (245738@main): <https://commits.webkit.org/245738@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 448365 [details].
Took it all. (In reply to Chris Dumez from comment #7) > Comment on attachment 448250 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=448250&action=review > > r=me > > > Source/WebKit/NetworkProcess/cocoa/WebSocketTaskCocoa.h:73 > > + const std::optional<WebCore::SecurityOriginData>& topOrigin() const { return m_topOrigin; } > > SecurityOriginData has an "empty" state so we could use a SecurityOriginData > as data members (instead of a std::optional<>) and have the call sites check > SecurityOriginData::isEmpty(). > > > Source/WebKit/NetworkProcess/cocoa/WebSocketTaskCocoa.mm:51 > > + if (clientOrigin.topOrigin == clientOrigin.clientOrigin) > > This could use a comment to explain why we only initialize m_topOrigin if > topOrigin is the same as clientOrigin. > > > Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:121 > > + MessageSender::send(Messages::NetworkConnectionToWebProcess::CreateSocketChannel { *request, protocol, m_identifier, m_webPageProxyID, ClientOrigin { m_document->topOrigin().data(), m_document->securityOrigin().data() } }); > > I bet we have this logic in several places and it may be worth having a > getter on Document that returns a ClientOrigin at this point.
*** Bug 233665 has been marked as a duplicate of this bug. ***