Break out WebSocket core features into new websocket-core components #3055
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…websocket Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com> # Conflicts: # Jenkinsfile # jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java # jetty-websocket/javax-websocket-client-impl/pom.xml # jetty-websocket/javax-websocket-server-impl/pom.xml # jetty-websocket/websocket-api/pom.xml # jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java # jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketRemoteEndpoint.java # jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java # jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/extensions/AbstractExtension.java # jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java # jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java # jetty-websocket/websocket-tests/src/main/java/org/eclipse/jetty/websocket/tests/LocalWebSocketConnection.java
…websocket Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com> # Conflicts: # Jenkinsfile # jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/ClientContainer.java # jetty-websocket/javax-websocket-server-impl/pom.xml # jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/AnnotatedServerEndpointConfig.java # jetty-websocket/websocket-api/pom.xml # jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java # jetty-websocket/websocket-server/pom.xml # jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java # jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java # jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/ServletUpgradeRequest.java
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
I think this test is trying to use QuotedCSV in the wrong way, as it often is offering values when it should be testing parameters. I have added some examples to show that QuotedCSV is capable of handling the quotes, spaces and embedded commas of the tests, but only for parameters and not for values. The question is, is this sufficient for websocket? Signed-off-by: Greg Wilkins <gregw@webtide.com>
The handling of a Close Frame may close the endpoint, so parsing should not continue Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
CookiesTest.testCookiesAreSentToClient was failing due to the Assertion being caught and logged by the upgrade request the response is now captured by a FuturePromise and the assertions are done in the main test the reason the assertion was failing was due to lowercase "set-cookie" instead of "Set-Cookie" Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Jetty 10.0.x Websocket Reimport This is a reimport of the refactored websockets for jetty10 (intended to replace the current implementation from jetty9). The work is based on the jetty-10.0.x-websocket branch and refined and completed the work started there, and thus is not as large a contribution as it may appear from the change count. This contribution is entirely from myself (@lachlan-roberts who has singed the Eclipse ECA for the jetty project), and eclipse jetty committers: @gregw, @joakime and @sbordet. The protocol elements of a websocket connection have been reworked into the websocket-core module. The jetty and javax API websocket modules are now implemented using the websocket-core. This PR passes the websocket tests and autobahn testsuite, but should be considered a work in progress that needs further testing within the main jetty repository and development process.
Signed-off-by: Greg Wilkins <gregw@webtide.com>
|
This PR is for the technical review before merging back to 10.0.x |
joakime
reviewed
Nov 1, 2018
joakime
reviewed
Nov 1, 2018
| @@ -795,6 +795,19 @@ public URI toURI() throws URISyntaxException | |||
| return new URI(_scheme,null,_host,_port,_path,_query==null?null:UrlEncoded.decodeString(_query),_fragment); | |||
| } | |||
|
|
|||
| /* ------------------------------------------------------------ */ | |||
| public URI asURI() | |||
There was a problem hiding this comment.
Do we really need this? isn't the existing toURI() sufficient?
There was a problem hiding this comment.
I'll remove the checked exception from toURI
joakime
reviewed
Nov 1, 2018
jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java
Show resolved
Hide resolved
joakime
reviewed
Nov 1, 2018
| // Bad URI (no scheme, no host, no path) | ||
| Arguments.of("//",null,null,null,null,null,null,null), | ||
| // Bad URI (no scheme, no host, no path) | ||
| // Arguments.of("//",null,null,null,null,null,null,null), |
There was a problem hiding this comment.
Move this one to the HttpURI testcase of "Bad" URIs?
joakime
reviewed
Nov 1, 2018
joakime
reviewed
Nov 1, 2018
| } | ||
|
|
||
| /* ------------------------------------------------------------ */ | ||
| /** Append bytes to a buffer. |
There was a problem hiding this comment.
Seems like a copy/paste flub.
Perhaps it should be "Append String UTF-8 bytes to a buffer" ?
joakime
reviewed
Nov 1, 2018
joakime
approved these changes
Nov 1, 2018
Signed-off-by: Greg Wilkins <gregw@webtide.com>
joakime
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A major refactor of websocket to be based around a core implementation