Skip to content

Commit

Permalink
Add ring.websocket/PingListener
Browse files Browse the repository at this point in the history
Fix Jetty adapter not responding correctly to pings.
  • Loading branch information
weavejester committed Sep 29, 2023
1 parent 5a5b8b9 commit af67081
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 7 deletions.
20 changes: 20 additions & 0 deletions SPEC-alpha.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,17 @@ protocol:
(on-close [listener socket code reason]))
```

It *may* optionally satisfy the `ring.websocket/PingListener` protocol:

```clojure
(defprotocol PingListener
(on-ping [listener socket data]))
```

If the `PingListener` protocol is not satisifed, the adapter *must*
default to respond to each ping message with a corresponding pong
message that has the same data.

#### on-open

Called once when the websocket is first opened. Supplies a `socket`
Expand All @@ -288,6 +299,15 @@ Called when a text or binary message frame is received from the client.
The `message` argument may be a `String` or a `java.nio.ByteBuffer`
depending on whether the message is text or binary.

#### on-ping

Called when a "ping" frame is received from the client. The `data`
argument is a `java.nio.ByteBuffer` that contains optional client
session data.

If the user implements this method, they are responsible for sending
the return "pong" that the websocket protocol expects.

#### on-pong

Called when a "pong" frame is received from the client. The `data`
Expand Down
22 changes: 17 additions & 5 deletions ring-core/src/ring/websocket.clj
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
(:import [java.nio ByteBuffer]))

(defprotocol Listener
"A protocol for handling websocket events. The second argument is
always an object that satisfies the Socket protocol."
"A protocol for handling websocket events. The second argument is always an
object that satisfies the Socket protocol."
(on-open [listener socket]
"Called when the websocket is opened.")
(on-message [listener socket message]
Expand All @@ -21,8 +21,17 @@
"Called when the websocket is closed, along with an integer code and a
plaintext string reason for being closed."))

(extend-protocol Listener
clojure.lang.IPersistentMap
(defprotocol PingListener
"A protocol for handling ping websocket events. The second argument is always
always an object that satisfies the Socket protocol. This is separate from
the Listener protocol as some APIs (for example Jakarta) don't support
listening for ping events."
(on-ping [listener socket data]
"Called when a ping is received from the client. The client may provide
additional binary data, represented by the data ByteBuffer."))

(extend-type clojure.lang.IPersistentMap
Listener
(on-open [m socket]
(when-let [kv (find m :on-open)] ((val kv) socket)))
(on-message [m socket message]
Expand All @@ -32,7 +41,10 @@
(on-error [m socket throwable]
(when-let [kv (find m :on-error)] ((val kv) socket throwable)))
(on-close [m socket code reason]
(when-let [kv (find m :on-close)] ((val kv) socket code reason))))
(when-let [kv (find m :on-close)] ((val kv) socket code reason)))
PingListener
(on-ping [m socket data]
(when-let [kv (find m :on-ping)] ((val kv) socket data))))

(defprotocol Socket
"A protocol for sending data via websocket."
Expand Down
2 changes: 1 addition & 1 deletion ring-jetty-adapter/project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
{:dev {:dependencies [[clj-http "3.12.3"]
[less-awful-ssl "1.0.6"]
[hato "0.9.0"]
[org.slf4j/slf4j-simple "2.0.9"]]
#_[org.slf4j/slf4j-simple "2.0.9"]]
:jvm-opts ["-Dorg.eclipse.jetty.server.HttpChannelState.DEFAULT_TIMEOUT=500"]}
:1.8 {:dependencies [[org.clojure/clojure "1.8.0"]]}
:1.9 {:dependencies [[org.clojure/clojure "1.9.0"]]}
Expand Down
5 changes: 4 additions & 1 deletion ring-jetty-adapter/src/ring/adapter/jetty.clj
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@
(let [buffer (ByteBuffer/wrap payload offset length)]
(ws/on-message listener @socket buffer)))
WebSocketPingPongListener
(onWebSocketPing [_ _])
(onWebSocketPing [_ payload]
(if (satisfies? ws/PingListener listener)
(ws/on-ping listener @socket payload)
(ws/pong @socket payload)))
(onWebSocketPong [_ payload]
(ws/on-pong listener @socket payload)))))

Expand Down
47 changes: 47 additions & 0 deletions ring-jetty-adapter/test/ring/adapter/test/jetty.clj
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,53 @@
(is (= [[:ping "foo"] [:pong "foo"]]
@log))))

(testing "ping pong from client"
(let [log (atom [])
handler (constantly
{::ws/listener
(reify ws/Listener
(on-open [_ _])
(on-message [_ _ _])
(on-pong [_ _ _])
(on-error [_ _ _])
(on-close [_ _ _ _]))})]
(with-server handler {:port test-port}
(let [ws @(hato/websocket test-websocket-url
{:on-pong
(fn [_ d]
(swap! log conj [:pong (buf->str d)]))})]
(Thread/sleep 100)
(hato/ping! ws (ByteBuffer/wrap (.getBytes "foo")))
@(hato/close! ws)
(Thread/sleep 100)))
(is (= [[:pong "foo"]] @log))))

(testing "custom on-ping"
(let [log (atom [])
handler (constantly
{::ws/listener
(reify ws/Listener
(on-open [_ _])
(on-message [_ _ _])
(on-pong [_ _ _])
(on-error [_ _ _])
(on-close [_ _ _ _])
ws/PingListener
(on-ping [_ sock data]
(ws/pong sock data)
(swap! log conj [:ping (buf->str data)])))})]
(with-server handler {:port test-port}
(let [ws @(hato/websocket test-websocket-url
{:on-pong
(fn [_ d]
(swap! log conj [:pong (buf->str d)]))})]
(Thread/sleep 100)
(hato/ping! ws (ByteBuffer/wrap (.getBytes "foo")))
@(hato/close! ws)
(Thread/sleep 100)))
(is (= [[:ping "foo"] [:pong "foo"]]
@log))))

(testing "open?"
(let [log (atom [])
handler (constantly
Expand Down

0 comments on commit af67081

Please sign in to comment.