Skip to content

[curl] CloseEvent has wasClean=true and no error code when the WebSocket connection got terminated from the server side#9728

Merged
webkit-early-warning-system merged 1 commit into
WebKit:mainfrom
fujii:eng/WinCairo-CloseEvent-has-wasCleantrue-and-no-error-code-when-the-WebSocket-connection-got-terminated-from-the-server-side
Feb 8, 2023
Merged

[curl] CloseEvent has wasClean=true and no error code when the WebSocket connection got terminated from the server side#9728
webkit-early-warning-system merged 1 commit into
WebKit:mainfrom
fujii:eng/WinCairo-CloseEvent-has-wasCleantrue-and-no-error-code-when-the-WebSocket-connection-got-terminated-from-the-server-side

Conversation

@fujii

@fujii fujii commented Feb 7, 2023

Copy link
Copy Markdown
Contributor

4284760

[curl] CloseEvent has wasClean=true and no error code when the WebSocket connection got terminated from the server side
https://bugs.webkit.org/show_bug.cgi?id=240227

Reviewed by Don Olmstead.

It should return a correct error code rather than 0.

* LayoutTests/http/tests/websocket/tests/hybi/server-close-2-expected.txt: Added.
* LayoutTests/http/tests/websocket/tests/hybi/server-close-2.html: Added.
* LayoutTests/http/tests/websocket/tests/hybi/server-close-2_wsh.py: Added.
* Source/WebKit/NetworkProcess/curl/WebSocketTaskCurl.cpp:
(WebKit::WebSocketTask::didReceiveData):
Use WebSocketChannel::CloseEventCode::CloseEventCodeAbnormalClosure if
the connection disconnected.

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

40465c6

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

@fujii
fujii requested a review from cdumez as a code owner February 7, 2023 01:50
@fujii fujii self-assigned this Feb 7, 2023
@fujii fujii added the Platform Portability improvements and other general platform improvements not driven directly by site bugs. label Feb 7, 2023
@webkit-early-warning-system

webkit-early-warning-system commented Feb 7, 2023

Copy link
Copy Markdown
Collaborator

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 7, 2023
@donny-dont

Copy link
Copy Markdown
Contributor

Don't land without figuring out test failures

@fujii fujii removed the merging-blocked Applied to prevent a change from being merged label Feb 8, 2023
@fujii
fujii force-pushed the eng/WinCairo-CloseEvent-has-wasCleantrue-and-no-error-code-when-the-WebSocket-connection-got-terminated-from-the-server-side branch from 5fff330 to 40465c6 Compare February 8, 2023 03:54
@webkit-early-warning-system

webkit-early-warning-system commented Feb 8, 2023

Copy link
Copy Markdown
Collaborator

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 8, 2023
@fujii fujii added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Feb 8, 2023
…ket connection got terminated from the server side

https://bugs.webkit.org/show_bug.cgi?id=240227

Reviewed by Don Olmstead.

It should return a correct error code rather than 0.

* LayoutTests/http/tests/websocket/tests/hybi/server-close-2-expected.txt: Added.
* LayoutTests/http/tests/websocket/tests/hybi/server-close-2.html: Added.
* LayoutTests/http/tests/websocket/tests/hybi/server-close-2_wsh.py: Added.
* Source/WebKit/NetworkProcess/curl/WebSocketTaskCurl.cpp:
(WebKit::WebSocketTask::didReceiveData):
Use WebSocketChannel::CloseEventCode::CloseEventCodeAbnormalClosure if
the connection disconnected.

Canonical link: https://commits.webkit.org/260025@main
@webkit-early-warning-system
webkit-early-warning-system force-pushed the eng/WinCairo-CloseEvent-has-wasCleantrue-and-no-error-code-when-the-WebSocket-connection-got-terminated-from-the-server-side branch from 40465c6 to 4284760 Compare February 8, 2023 20:03
@webkit-early-warning-system
webkit-early-warning-system merged commit 4284760 into WebKit:main Feb 8, 2023
@webkit-commit-queue

Copy link
Copy Markdown
Collaborator

Committed 260025@main (4284760): https://commits.webkit.org/260025@main

Reviewed commits have been landed. Closing PR #9728 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 Feb 8, 2023
webkit-commit-queue pushed a commit to annevk/WebKit that referenced this pull request Jul 24, 2023
https://bugs.webkit.org/show_bug.cgi?id=255872
rdar://108738795

Reviewed by Darin Adler.

It turns out that JSON, HTTP, and XML all use the same whitespace
definition, so let's make them share it. Also correct an existing
comment for that function as \v is not part of isASCIIWhitespace(), but
\f is.

Furthermore, remove the "optimization" from these whitespace functions
per a comment from Chris Dumez at
WebKit#13080 (comment):

> Just verified out of curiosity and llvm does generate the same code
> with -O2 (tried both arm64 and x86_64):
>
> isXMLSpace1(char):                       // @isXMLSpace1(char)
>         mov     x8, WebKit#9728                       // =0x2600
>         and     w9, w0, #0xff
>         movk    x8, WebKit#1, lsl WebKit#32
>         cmp     w9, WebKit#33
>         cset    w9, lo
>         lsr     x8, x8, x0
>         and     w0, w9, w8
>         ret
> isXMLSpace2(char):                       // @isXMLSpace2(char)
>         mov     x8, WebKit#9728                       // =0x2600
>         and     w9, w0, #0xff
>         movk    x8, WebKit#1, lsl WebKit#32
>         cmp     w9, WebKit#33
>         cset    w9, lo
>         lsr     x8, x8, x0
>         and     w0, w9, w8
>         ret
>
> Ahmad-S792 Let's simplify the code then.

* Source/WTF/wtf/ASCIICType.h:
(WTF::isASCIIWhitespace):
(WTF::isJSONOrHTTPOrXMLWhitespace):
(WTF::isJSONOrHTTPWhitespace): Deleted.
* Source/WTF/wtf/JSONValues.cpp:
(WTF::JSONImpl::Value::parseJSON):
* Source/WTF/wtf/text/StringToIntegerConversion.h:
* Source/WebCore/Modules/cache/DOMCache.cpp:
(WebCore::hasResponseVaryStarHeaderValue):
* Source/WebCore/Modules/cache/DOMCacheEngine.cpp:
(WebCore::DOMCacheEngine::queryCacheMatch):
* Source/WebCore/Modules/fetch/FetchBodyConsumer.cpp:
(WebCore::parseParameters):
(WebCore::parseMIMEType):
(WebCore::FetchBodyConsumer::packageFormData):
* Source/WebCore/Modules/fetch/FetchHeaders.cpp:
(WebCore::canWriteHeader):
(WebCore::appendToHeaderMap):
(WebCore::FetchHeaders::set):
(WebCore::FetchHeaders::filterAndFill):
* Source/WebCore/mathml/MathMLPresentationElement.cpp:
(WebCore::MathMLPresentationElement::parseMathMLLength):
* Source/WebCore/mathml/MathMLTokenElement.cpp:
(WebCore::MathMLTokenElement::convertToSingleCodePoint):
* Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp:
(WebCore::ContentSecurityPolicyDirectiveList::parse):
* Source/WebCore/platform/ReferrerPolicy.cpp:
(WebCore::parseReferrerPolicy):
* Source/WebCore/platform/network/DataURLDecoder.cpp:
(WebCore::DataURLDecoder::DecodeTask::process):
* Source/WebCore/platform/network/HTTPParsers.cpp:
(WebCore::parseContentTypeOptionsHeader):
(WebCore::parseClearSiteDataHeader):
(WebCore::parseRange):
(WebCore::parseCrossOriginResourcePolicyHeader):
* Source/WebCore/platform/network/HTTPParsers.h:
(WebCore::addToAccessControlAllowList):
* Source/WebCore/platform/network/ParsedContentType.cpp:
(WebCore::skipSpaces):
(WebCore::parseToken):
(WebCore::ParsedContentType::create):
(WebCore::ParsedContentType::setContentType):
* Source/WebCore/platform/network/ResourceResponseBase.cpp:
(WebCore::ResourceResponseBase::containsInvalidHTTPHeaders const):
* Source/WebCore/platform/network/TimingAllowOrigin.cpp:
(WebCore::passesTimingAllowOriginCheck):
* Source/WebCore/xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::setRequestHeader):
* Source/WebCore/xml/XPathFunctions.cpp:
(WebCore::XPath::FunNormalizeSpace::evaluate const):
* Source/WebCore/xml/XPathParser.cpp:
(WebCore::XPath::Parser::skipWS):
* Source/WebCore/xml/XPathUtil.cpp:
(WebCore::XPath::isXMLSpace): Deleted.
* Source/WebCore/xml/XPathUtil.h:
* Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:
(WebKit::CacheStorage::updateVaryInformation):
* Source/WebKit/NetworkProcess/soup/WebSocketTaskSoup.cpp:
(WebKit::WebSocketTask::WebSocketTask):
* Source/WebKit/NetworkProcess/storage/CacheStorageRecord.h:
(WebKit::CacheStorageRecordInformation::updateVaryHeaders):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::shouldSkipDecidePolicyForResponse const):

Canonical link: https://commits.webkit.org/266253@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Platform Portability improvements and other general platform improvements not driven directly by site bugs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants