From 90e7307fb4ce79d5a4f0a3a96334492726fdf862 Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Fri, 19 Dec 2025 21:12:37 +0000 Subject: [PATCH] Fix websocket_closed not always being queued on failure - Reorganise the HTTP test code to make it a bit more extensible. Add support for sending messages to connected websockets. - Provide a friendlier message for too-large-payload errors. - Return failure reason from Websocket.receive Fixes #2149. --- .../core/apis/http/NetworkUtils.java | 6 ++ .../apis/http/websocket/WebsocketHandle.java | 7 +- .../apis/http/websocket/WebsocketHandler.java | 3 +- .../core/apis/http/HttpServer.kt | 79 +++++++++++-------- .../core/apis/http/TestHttpApi.kt | 45 ++++++++--- 5 files changed, 94 insertions(+), 46 deletions(-) diff --git a/projects/core/src/main/java/dan200/computercraft/core/apis/http/NetworkUtils.java b/projects/core/src/main/java/dan200/computercraft/core/apis/http/NetworkUtils.java index 594dfccfd..ed3137c82 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/apis/http/NetworkUtils.java +++ b/projects/core/src/main/java/dan200/computercraft/core/apis/http/NetworkUtils.java @@ -17,6 +17,8 @@ import io.netty.channel.nio.NioEventLoopGroup; import io.netty.channel.socket.SocketChannel; import io.netty.handler.codec.DecoderException; import io.netty.handler.codec.TooLongFrameException; +import io.netty.handler.codec.http.websocketx.CorruptedWebSocketFrameException; +import io.netty.handler.codec.http.websocketx.WebSocketCloseStatus; import io.netty.handler.codec.http.websocketx.WebSocketHandshakeException; import io.netty.handler.proxy.HttpProxyHandler; import io.netty.handler.proxy.Socks4ProxyHandler; @@ -245,6 +247,10 @@ public final class NetworkUtils { return "Timed out"; } else if (cause instanceof SSLHandshakeException || (cause instanceof DecoderException && cause.getCause() instanceof SSLHandshakeException)) { return "Could not create a secure connection"; + } else if (cause instanceof CorruptedWebSocketFrameException e) { + return e.closeStatus() == WebSocketCloseStatus.MESSAGE_TOO_BIG + ? "Received a too-large message" + : "Corrupted websocket message"; } else { return "Could not connect"; } diff --git a/projects/core/src/main/java/dan200/computercraft/core/apis/http/websocket/WebsocketHandle.java b/projects/core/src/main/java/dan200/computercraft/core/apis/http/websocket/WebsocketHandle.java index f42a99516..25ff6ed92 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/apis/http/websocket/WebsocketHandle.java +++ b/projects/core/src/main/java/dan200/computercraft/core/apis/http/websocket/WebsocketHandle.java @@ -57,8 +57,11 @@ public class WebsocketHandle { * @cc.treturn [1] string The received message. * @cc.treturn boolean If this was a binary message. * @cc.treturn [2] nil If the websocket was closed while waiting, or if we timed out. + * @cc.treturn [2] string The reason we failed to receive a message. Either the reason the websocket was closed + * (as returned by [`websocket_closed`], or the string {@code "Timed out"}. * @cc.changed 1.80pr1.13 Added return value indicating whether the message was binary. * @cc.changed 1.87.0 Added timeout argument. + * @cc.changed 1.117.0 Added return value indicating why receiving the message failed. */ @LuaFunction public final MethodResult receive(Optional timeout) throws LuaException { @@ -155,11 +158,11 @@ public class WebsocketHandle { } else if (event.length >= 2 && Objects.equals(event[0], CLOSE_EVENT) && Objects.equals(event[1], address) && websocket.isClosed()) { // If the socket is closed abort. environment.cancelTimer(timeoutId); - return MethodResult.of(); + return MethodResult.of(null, event.length > 2 ? event[2] : "Connection closed"); } else if (event.length >= 2 && timeoutId != -1 && Objects.equals(event[0], TIMER_EVENT) && event[1] instanceof Number id && id.intValue() == timeoutId) { // If we received a matching timer event then abort. - return MethodResult.of(); + return MethodResult.of(null, "Timed out"); } return pull; diff --git a/projects/core/src/main/java/dan200/computercraft/core/apis/http/websocket/WebsocketHandler.java b/projects/core/src/main/java/dan200/computercraft/core/apis/http/websocket/WebsocketHandler.java index 3ff9fd721..924ffebfa 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/apis/http/websocket/WebsocketHandler.java +++ b/projects/core/src/main/java/dan200/computercraft/core/apis/http/websocket/WebsocketHandler.java @@ -71,9 +71,8 @@ class WebsocketHandler extends SimpleChannelInboundHandler { @Override public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) { - ctx.close(); - fail(NetworkUtils.toFriendlyError(cause)); + ctx.close(); } private void fail(String message) { diff --git a/projects/core/src/test/kotlin/dan200/computercraft/core/apis/http/HttpServer.kt b/projects/core/src/test/kotlin/dan200/computercraft/core/apis/http/HttpServer.kt index 8c45dbc69..ecb7df618 100644 --- a/projects/core/src/test/kotlin/dan200/computercraft/core/apis/http/HttpServer.kt +++ b/projects/core/src/test/kotlin/dan200/computercraft/core/apis/http/HttpServer.kt @@ -20,38 +20,49 @@ import io.netty.handler.codec.http.websocketx.extensions.compression.WebSocketSe import java.net.InetSocketAddress import java.nio.charset.StandardCharsets -/** - * Runs a small HTTP server to run alongside [TestHttpApi] - */ -object HttpServer { - fun runServer(run: (port: Int, stop: () -> Unit) -> Unit) { - val workerGroup: EventLoopGroup = NioEventLoopGroup(2) - try { - val ch = ServerBootstrap() - .group(workerGroup) - .channel(NioServerSocketChannel::class.java) - .childHandler( - object : ChannelInitializer() { - override fun initChannel(ch: SocketChannel) { - val p: ChannelPipeline = ch.pipeline() - p.addLast(HttpServerCodec()) - p.addLast(HttpContentCompressor()) - p.addLast(HttpObjectAggregator(8192)) - p.addLast(HttpServerHandler()) - p.addLast(WebSocketServerCompressionHandler()) - p.addLast(WebSocketServerProtocolHandler("/ws", null, true)) - p.addLast(WebSocketFrameHandler()) - } - }, - ).bind(0).sync().channel() - val port = (ch.localAddress() as InetSocketAddress).port +class HttpServer(val port: Int, private val workerGroup: EventLoopGroup, private val activeConnections: Set) { + /** Stop the server from running */ + fun stop() { + workerGroup.shutdownGracefully() + } + + /** Broadcast this message to every connected websocket */ + fun broadcast(message: WebSocketFrame) { + for (chan in activeConnections) chan.writeAndFlush(message) + } + + companion object { + /** Runs a small HTTP server to run alongside [TestHttpApi] */ + fun runServer(run: (server: HttpServer) -> Unit) { + val workerGroup: EventLoopGroup = NioEventLoopGroup(2) + val activeConnections = mutableSetOf() try { - run(port) { workerGroup.shutdownGracefully() } + val ch = ServerBootstrap() + .group(workerGroup) + .channel(NioServerSocketChannel::class.java) + .childHandler( + object : ChannelInitializer() { + override fun initChannel(ch: SocketChannel) { + val p: ChannelPipeline = ch.pipeline() + p.addLast(HttpServerCodec()) + p.addLast(HttpContentCompressor()) + p.addLast(HttpObjectAggregator(8192)) + p.addLast(HttpServerHandler()) + p.addLast(WebSocketServerCompressionHandler()) + p.addLast(WebSocketServerProtocolHandler("/ws", null, true)) + p.addLast(WebSocketFrameHandler(activeConnections)) + } + }, + ).bind(0).sync().channel() + val port = (ch.localAddress() as InetSocketAddress).port + try { + run(HttpServer(port, workerGroup, activeConnections)) + } finally { + ch.close().sync() + } } finally { - ch.close().sync() + workerGroup.shutdownGracefully().get() } - } finally { - workerGroup.shutdownGracefully().get() } } } @@ -111,7 +122,7 @@ private class HttpServerHandler : SimpleChannelInboundHandler() /** * A basic WS server which just sends back the original message. */ -private class WebSocketFrameHandler : SimpleChannelInboundHandler() { +private class WebSocketFrameHandler(private val activeConnections: MutableSet) : SimpleChannelInboundHandler() { override fun channelRead0(ctx: ChannelHandlerContext, frame: WebSocketFrame) { if (frame is TextWebSocketFrame) { // Send the uppercase string back. @@ -124,10 +135,16 @@ private class WebSocketFrameHandler : SimpleChannelInboundHandler + runServer { server -> LuaTaskRunner.runTest { - val url = "http://127.0.0.1:$port" + val url = "http://127.0.0.1:${server.port}" val httpApi = addApi(HTTPAPI(environment)) assertThat("http.request succeeded", httpApi.request(ObjectArguments(url)), array(equalTo(true))) @@ -66,9 +68,9 @@ class TestHttpApi { @Test fun `Connects to websocket`() { - runServer { port, _ -> + runServer { server -> LuaTaskRunner.runTest { - val url = "ws://127.0.0.1:$port/ws" + val url = "ws://127.0.0.1:${server.port}/ws" val httpApi = addApi(HTTPAPI(environment)) assertThat("http.websocket succeeded", httpApi.websocket(ObjectArguments(url)), array(equalTo(true))) @@ -91,9 +93,9 @@ class TestHttpApi { @Test fun `Errors if too many websocket messages are sent`() { - runServer { port, _ -> + runServer { server -> LuaTaskRunner.runTest { - val url = "ws://127.0.0.1:$port/ws" + val url = "ws://127.0.0.1:${server.port}/ws" val httpApi = addApi(HTTPAPI(environment)) assertThat("http.websocket succeeded", httpApi.websocket(ObjectArguments(url)), array(equalTo(true))) @@ -115,10 +117,31 @@ class TestHttpApi { } @Test - fun `Queues an event when the socket is externally closed`() { - runServer { port, stop -> + fun `Closes if a websocket message is too large`() { + runServer { server -> LuaTaskRunner.runTest { - val url = "ws://127.0.0.1:$port/ws" + val url = "ws://127.0.0.1:${server.port}/ws" + val httpApi = addApi(HTTPAPI(environment)) + assertThat("http.websocket succeeded", httpApi.websocket(ObjectArguments(url)), array(equalTo(true))) + + val connectEvent = pullEvent() + assertThat(connectEvent, array(equalTo("websocket_success"), equalTo(url), isA(WebsocketHandle::class.java))) + + val out = ByteArray(AddressRule.WEBSOCKET_MESSAGE + 1) + Random(0xDEADBEEF).nextBytes(out) + server.broadcast(BinaryWebSocketFrame(Unpooled.wrappedBuffer(out))) + + val closeEvent = pullEvent() + assertThat(closeEvent, array(equalTo("websocket_closed"), equalTo(url), equalTo("Received a too-large message"), nullValue())) + } + } + } + + @Test + fun `Queues an event when the socket is externally closed`() { + runServer { server -> + LuaTaskRunner.runTest { + val url = "ws://127.0.0.1:${server.port}/ws" val httpApi = addApi(HTTPAPI(environment)) assertThat("http.websocket succeeded", httpApi.websocket(ObjectArguments(url)), array(equalTo(true))) @@ -127,7 +150,7 @@ class TestHttpApi { val websocket = connectEvent[2] as WebsocketHandle - stop() + server.stop() val closeEvent = pullEvent("websocket_closed") assertThat(