jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.86k stars 1.91k forks source link

WebSocket client does not receive messages before sending a message by itself #3147

Closed sidamos closed 5 years ago

sidamos commented 5 years ago

Clients only get messages from the server after they have send a message themselves. This happens with browser and Java clients.

Repro:

Normally, messages from the Java client should appear in the browser console. Sometimes it works, sometimes not. It mostly does not work if the server is already running and the web page is being reloaded with F5. Similar thing happens to the Java client if I comment the Hello message and the message from the loop, so that it does not send messages. Then if I send a message from the browser client (click Send button), the Java client does not receive it.

Workaround: Clients have to send a message within the onOpen callback. Then they are receiving messages from the server reliably.

We encountered this problem in our application, the attached repro is the slightly modified demo code from https://github.com/reta/jetty-web-sockets-jsr356.

In our application, we are using embedded Jetty. When we switch to embedded Tomcat 7 (or deploy on production Tomcat 8), then it works without the workaround above. So, I guess it is a bug in Jetty.

Using Java 8 on Windows 7 (running on localhost).

jetty-web-sockets-jsr356.zip websocket.zip

joakime commented 5 years ago

Your code can be found at https://github.com/joakime/jetty-issue-3147 with one extra commit to fix your JSR356 usage.

See extra commit https://github.com/joakime/jetty-issue-3147/commit/e24d1e8b9d881ca9be38a723930501b8b7d4ef76

It works as expected on Windows 10 using Java 8u181.

Also of note, Spring Framework 3.2.5.RELEASE has many security vulnerabilities, you should consider upgrading to Spring 4.3.17 or newer ASAP.

CVE-2016-5007 moderate severity Vulnerable versions: < 4.3.1 Patched version: 4.3.1 Both Spring Security 3.2.x, 4.0.x, 4.1.0 and the Spring Framework 3.2.x, 4.0.x, 4.1.x, 4.2.x rely on URL pattern mappings for authorization and for mapping requests to controllers respectively. Differences in the strictness of the pattern matching mechanisms, for example with regards to space trimming in path segments, can lead Spring Security to not recognize certain paths as not protected that are in fact mapped to Spring MVC controllers that should be protected. The problem is compounded by the fact that the Spring Framework provides richer features with regards to pattern matching as well as by the fact that pattern matching in each Spring Security and the Spring Framework can easily be customized creating additional differences.

CVE-2015-5211 high severity Vulnerable versions: > 3.2.0, < 3.2.15 Patched version: 3.2.15 Under some situations, the Spring Framework 4.2.0 to 4.2.1, 4.0.0 to 4.1.7, 3.2.0 to 3.2.14 and older unsupported versions is vulnerable to a Reflected File Download (RFD) attack. The attack involves a malicious user crafting a URL with a batch script extension that results in the response being downloaded rather than rendered and also includes some input reflected in the response.

CVE-2015-3192 moderate severity Vulnerable versions: < 3.2.14 Patched version: 3.2.14 Pivotal Spring Framework before 3.2.14 and 4.x before 4.1.7 do not properly process inline DTD declarations when DTD is not entirely disabled, which allows remote attackers to cause a denial of service (memory consumption and out-of-memory errors) via a crafted XML file.

CVE-2018-1275 high severity Vulnerable versions: < 4.3.16 Patched version: 4.3.16 Spring Framework, versions 5.0 prior to 5.0.5 and versions 4.3 prior to 4.3.16 and older unsupported versions, allow applications to expose STOMP over WebSocket endpoints with a simple, in-memory STOMP broker through the spring-messaging module. A malicious user (or attacker) can craft a message to the broker that can lead to a remote code execution attack. This CVE addresses the partial fix for CVE-2018-1270 in the 4.3.x branch of the Spring Framework.

CVE-2018-1272 moderate severity Vulnerable versions: < 4.3.15 Patched version: 4.3.15 Spring Framework, versions 5.0 prior to 5.0.5 and versions 4.3 prior to 4.3.15 and older unsupported versions, provide client-side support for multipart requests. When Spring MVC or Spring WebFlux server application (server A) receives input from a remote client, and then uses that input to make a multipart request to another server (server B), it can be exposed to an attack, where an extra multipart is inserted in the content of the request from server A, causing server B to use the wrong value for a part it expects. This could to lead privilege escalation, for example, if the part content represents a username or user roles.

CVE-2018-1271 moderate severity Vulnerable versions: < 4.3.15 Patched version: 4.3.15 Spring Framework, versions 5.0 prior to 5.0.5 and versions 4.3 prior to 4.3.15 and older unsupported versions, allow applications to configure Spring MVC to serve static resources (e.g. CSS, JS, images). When static resources are served from a file system on Windows (as opposed to the classpath, or the ServletContext), a malicious user can send a request using a specially crafted URL that can lead a directory traversal attack.

CVE-2018-1270 high severity Vulnerable versions: < 4.3.16 Patched version: 4.3.16 Spring Framework, versions 5.0 prior to 5.0.5 and versions 4.3 prior to 4.3.15 and older unsupported versions, allow applications to expose STOMP over WebSocket endpoints with a simple, in-memory STOMP broker through the spring-messaging module. A malicious user (or attacker) can craft a message to the broker that can lead to a remote code execution attack.

CVE-2018-1257 moderate severity Vulnerable versions: < 4.3.17 Patched version: 4.3.17 Spring Framework, versions 5.0.x prior to 5.0.6, versions 4.3.x prior to 4.3.17, and older unsupported versions allows applications to expose STOMP over WebSocket endpoints with a simple, in-memory STOMP broker through the spring-messaging module. A malicious user (or attacker) can craft a message to the broker that can lead to a regular expression, denial of service attack.

CVE-2016-9878 moderate severity Vulnerable versions: < 3.2.18 Patched version: 3.2.18 An issue was discovered in Pivotal Spring Framework before 3.2.18, 4.2.x before 4.2.9, and 4.3.x before 4.3.5. Paths provided to the ResourceServlet were not properly sanitized and as a result exposed to directory traversal attacks.

sidamos commented 5 years ago

Just a side note: The repro code, as I said, is borrowed from https://github.com/reta/jetty-web-sockets-jsr356, which is not our code. I just used it as a base for repro code for this issue because it is a small and complete example. Our own (big) application uses a recent version of Spring.

If fact, we used https://technology.amis.nl/2015/05/14/java-web-application-sending-json-messages-through-websocket-to-html5-browser-application-for-real-time-push as inspiration for our implementation. So, there are more examples on the web for doing it wrong, if I understand your statements correctly. They do however work fine with Tomcat.

Having said that, your changes do not fix the issue.

  1. Test with browser client Run ServerStarter and ClientStarter. Java client is receiving its own messages alright. Now load provided sample web client. If we are lucky, it will receive messages after load. After a reload of the page, receiving stops. The Java server still thinks it is sending messages to both clients (Java and browser), but the browser client receives nothing.

  2. Test with Java client Comment line 24 in ClientStarter.java. Run ServerStarter and ClientStarter. Load web client and press Send button several times. The Java server still thinks it is sending messages to both clients (Java and browser), but the Java client receives nothing.

joakime commented 5 years ago

Feedback on your original submitted code. It seems to only use onClose() to remove a Session. It skips / ignores onError() to remove a Session also (such as what happens when you refresh a browser client).

The knowledge of what Sessions are open is a mandated part of the JSR-356 spec, don't reimplement it with your own collection of Sessions.

Your scenario 2 works as expected for me, with no issues. Commented out line 24 and the Java server still correctly sees the termination of WebSockets and the Java client sees the messages properly.

As for scenario 1, here's my feedback.

What I'm using ... On Chrome 70.0.3538.102 (Official Build) (64-bit) on Windows 10 Oracle OpenJDK 1.8.0 update 181

When testing on Chrome, and refreshing the page, or navigating away, or simply just putting the tab in the background (yes, the javascript websocket client will terminate if its not in the foreground), the Javascript WebSocket client terminates. How it terminates is quite varied.

  1. Sometimes It sends a WebSocket close code 1001 (Going Away).
  2. Sometimes It sends a WebSocket close without status or reason.
  3. Sometimes the TCP connection is abruptly terminated.
  4. Sometimes the TCP connection sticks around until it times out.
  5. Sometimes the TCP connection sticks around until its received enough messages to trigger standard congestion behaviors.

In cases 1, 2, 3 the server notices quickly. In case 1, and 2 the onClose() would be the terminal closure and you can remove the session then. In case 3, 4, and 5 the onError() would be the terminal closure and you can remove the session then. In case 4, and 5, the size of your TCP buffers on your machine, and the idle configuration determine when you will see that closure.

Tomcat is still using old school blocking Socket connections and will see case 4 and 5 on loopback slightly quicker then Jetty. Jetty has no blocking Socket connection behavior, and uses Java's NIO exclusively, and as such will not use resources if nothing is happening (networking wise).

BTW, this situation is why WebSocket has PING/PONG messages. Your application would use that to determine if the other side is really there (or not)

If you are tasked with writing a communication among many WebSocket clients, consider using a library for that, such as cometd, they've got all of this knowledge integrated into their libraries already. They understand the browser behavior, work with javascript, works with mobile apps (and mobile browsers), native applications, applications between multiple servers, cloud / clustered servers as well.

sidamos commented 5 years ago

Thanks for the insights.

Scenario 2: I do not understand why scenario 2 works for you. It does not work for me with your changed code.

About onError(): Even if removing the session in there, it still will not make messages go through to the browser after refresh, right? But I am already changing our code, so that Jetty is keeping track of the sessions, like you did it in your changes, so no need to remove sessions for us.

Scenario 1: Are you saying that it also does not work reliably for you with your changed code? If so, is then the idea of sending a message to the server in onOpen() a viable thing to do? Ar least for me it makes it reliable.

joakime commented 5 years ago

scenario 2 works without issue for me.

scenario 1 works reliably as well, all 5 possible closure use cases from a browser work too. some faster then others, but they do work.

I updated the code to allow me to specify a clientID in the messages that are sent (makes it easier to see what's going on).

You can start the clients with an optional command line argument specifying the client ID. And the websocket.html can use an optional clientId query parameter.

So I started the following ...

  1. ServerStarter (as-is)
  2. ClientStarter ONE
  3. ClientStarter TWO
  4. ClientStarter THREE
  5. Browser at .../websocket.html?clientId=UNO
  6. Browser at .../websocket.html?clientId=DOS

Then any combination of open close, navigate away, stop, start, kill, harsh, graceful, etc just behaves as expected. Each connected Server Endpoint takes an incoming message and iterates through to the other Sessions to send that same message. Each connected Client WebSocket endpoint (be it java or browser) sees the messages from the other clients.

What you describe isn't reproducible. Do you have some kind of firewall / security software on your old Windows 7 computer that is getting in the way with your testing perhaps?

sidamos commented 5 years ago

I do have the standard Windows firewall, that is all. Windows 7 is still supported for corporations AFAIK. Using Java 8u191 (also tested 8u121).

With the updated code, I am testing scenario 2:

Scenario 1:

Now it gets interesting: I tested the same things on Ubuntu 16.04.5 LTS with Java 8u191 inside VirtualBox that runs on my Windows 7 machine. Result: Everything works fine, like you described.

I do not have a Windows 10 machine to test right now but will probably have one in a few weeks.

If you want, you can close this issue since I have a workaround for Windows 7. OTOH, it should also work on Windows 7 out of the box IMHO.

sidamos commented 5 years ago

Additional info: Just tested both scenarios on another Windows 7 machine (from a colleague). Same as for me. Both scenarios do not work without my workaround.

joakime commented 5 years ago

Scenario 2: Did what you described. Java Client gets messages as normal.

issue-3139-proof

Scenario 1: Did what you described. Web Client reconnects and sees messages from Java Client.

issue-3139-proof-scenario1

This is on Windows 10, Java 8 update 181.

Also of note, the above described behavior is the same on Ubuntu 16.04.5, Fedora Core 28, and OSX 10.12.6 (tested on those as well).

sidamos commented 5 years ago

I believe you. ;-)

As I said, it works on Ubuntu as well for me. So, it only has problems on Windows 7, tested with 2 machines.

I'll leave it to the Jetty team if they want to make it work on Windows 7.

joakime commented 5 years ago

Testing on Windows 7 VM from https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/

Installed OpenJDK 8u192. Copied over compiled classes/libs from the test project.

Testing of both Scenarios.

Result: Same as Windows 10 with Java 8u181. There's no reproduction case still.

Next step, pull out an old laptop with Windows 7 and try again. So far, there's nothing wrong, and there's nothing to fix.

joakime commented 5 years ago

Same results on Windows 7 - SP1 on an old Lenovo Y550P I had here.

Oracle OpenJDK 8u192. Chrome 51 !! (yes, it worked on this old version, before Chrome decided it wanted to update itself)

Still have no reproduction case. It looks like something odd on your environments (some kind of corporate networking policy on your machines that is getting in the way?).

Closing as not reproducible, nothing for Jetty to do here. If you have an update, I'll be happy to reopen.