Merge branch 'mc-1.16.x' into mc-1.18.x

gradle/libs.versions.toml

@@ -7,7 +7,7 @@ parchment = "2022.03.13"
 parchmentMc = "1.18.2"
 
 autoService = "1.0.1"
-cobalt = { strictly = "[0.5.7,0.6.0)", prefer = "0.5.7" }
+cobalt = { strictly = "[0.5.8,0.6.0)", prefer = "0.5.8" }
 jetbrainsAnnotations = "23.0.0"
 kotlin = "1.7.10"
 kotlin-coroutines = "1.6.0"

src/main/java/dan200/computercraft/ComputerCraft.java

@@ -15,6 +15,7 @@
 import org.slf4j.LoggerFactory;
 
 import java.util.List;
+import java.util.OptionalInt;
 import java.util.concurrent.TimeUnit;
 
 @Mod( ComputerCraft.MOD_ID )
@@ -37,8 +38,8 @@ public final class ComputerCraft
     public static boolean httpEnabled = true;
     public static boolean httpWebsocketEnabled = true;
     public static List<AddressRule> httpRules = List.of(
-        AddressRule.parse( "$private", null, Action.DENY.toPartial() ),
-        AddressRule.parse( "*", null, Action.ALLOW.toPartial() )
+        AddressRule.parse( "$private", OptionalInt.empty(), Action.DENY.toPartial() ),
+        AddressRule.parse( "*", OptionalInt.empty(), Action.ALLOW.toPartial() )
     );
 
     public static int httpMaxRequests = 16;

src/main/java/dan200/computercraft/core/apis/http/options/Action.java

@@ -6,13 +6,17 @@
 package dan200.computercraft.core.apis.http.options;
 
 import javax.annotation.Nonnull;
+import java.util.OptionalInt;
+import java.util.OptionalLong;
 
 public enum Action
 {
     ALLOW,
     DENY;
 
-    private final PartialOptions partial = new PartialOptions( this, null, null, null, null );
+    private final PartialOptions partial = new PartialOptions(
+        this, OptionalLong.empty(), OptionalLong.empty(), OptionalInt.empty(), OptionalInt.empty()
+    );
 
     @Nonnull
     public PartialOptions toPartial()

src/main/java/dan200/computercraft/core/apis/http/options/AddressRule.java

@@ -16,6 +16,7 @@
 import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
+import java.util.OptionalInt;
 import java.util.regex.Pattern;
 
 /**
@@ -29,10 +30,10 @@ public final class AddressRule
     public static final int WEBSOCKET_MESSAGE = 128 * 1024;
 
     private final AddressPredicate predicate;
-    private final Integer port;
+    private final OptionalInt port;
     private final PartialOptions partial;
 
-    private AddressRule( @Nonnull AddressPredicate predicate, @Nullable Integer port, @Nonnull PartialOptions partial )
+    private AddressRule( @Nonnull AddressPredicate predicate, OptionalInt port, @Nonnull PartialOptions partial )
     {
         this.predicate = predicate;
         this.partial = partial;
@@ -40,7 +41,7 @@ private AddressRule( @Nonnull AddressPredicate predicate, @Nullable Integer port
     }
 
     @Nullable
-    public static AddressRule parse( String filter, @Nullable Integer port, @Nonnull PartialOptions partial )
+    public static AddressRule parse( String filter, OptionalInt port, @Nonnull PartialOptions partial )
     {
         int cidr = filter.indexOf( '/' );
         if( cidr >= 0 )
@@ -72,7 +73,7 @@ else if( filter.equalsIgnoreCase( "$private" ) )
      */
     private boolean matches( String domain, int port, InetAddress address, Inet4Address ipv4Address )
     {
-        if( this.port != null && this.port != port ) return false;
+        if( this.port.isPresent() && this.port.getAsInt() != port ) return false;
         return predicate.matches( domain )
             || predicate.matches( address )
             || (ipv4Address != null && predicate.matches( ipv4Address ));
@@ -80,8 +81,7 @@ private boolean matches( String domain, int port, InetAddress address, Inet4Addr
 
     public static Options apply( Iterable<? extends AddressRule> rules, String domain, InetSocketAddress socketAddress )
     {
-        PartialOptions options = null;
-        boolean hasMany = false;
+        PartialOptions options = PartialOptions.DEFAULT;
 
         int port = socketAddress.getPort();
         InetAddress address = socketAddress.getAddress();
@@ -91,24 +91,9 @@ public static Options apply( Iterable<? extends AddressRule> rules, String domai
         for( AddressRule rule : rules )
         {
             if( !rule.matches( domain, port, address, ipv4Address ) ) continue;
-
-            if( options == null )
-            {
-                options = rule.partial;
-            }
-            else
-            {
-
-                if( !hasMany )
-                {
-                    options = options.copy();
-                    hasMany = true;
-                }
-
-                options.merge( rule.partial );
-            }
+            options = options.merge( rule.partial );
         }
 
-        return (options == null ? PartialOptions.DEFAULT : options).toOptions();
+        return options.toOptions();
     }
 }

src/main/java/dan200/computercraft/core/apis/http/options/AddressRuleConfig.java

@@ -14,6 +14,8 @@
 import javax.annotation.Nullable;
 import java.util.Locale;
 import java.util.Optional;
+import java.util.OptionalInt;
+import java.util.OptionalLong;
 import java.util.concurrent.ConcurrentHashMap;
 
 /**
@@ -48,7 +50,7 @@ public static UnmodifiableConfig makeRule( String host, Action action )
     public static boolean checkRule( UnmodifiableConfig builder )
     {
         String hostObj = get( builder, "host", String.class ).orElse( null );
-        Integer port = get( builder, "port", Number.class ).map( Number::intValue ).orElse( null );
+        OptionalInt port = unboxOptInt( get( builder, "port", Number.class ) );
         return hostObj != null && checkEnum( builder, "action", Action.class )
             && check( builder, "port", Number.class )
             && check( builder, "timeout", Number.class )
@@ -65,11 +67,11 @@ public static AddressRule parseRule( UnmodifiableConfig builder )
         if( hostObj == null ) return null;
 
         Action action = getEnum( builder, "action", Action.class ).orElse( null );
-        Integer port = get( builder, "port", Number.class ).map( Number::intValue ).orElse( null );
-        Integer timeout = get( builder, "timeout", Number.class ).map( Number::intValue ).orElse( null );
-        Long maxUpload = get( builder, "max_upload", Number.class ).map( Number::longValue ).orElse( null );
-        Long maxDownload = get( builder, "max_download", Number.class ).map( Number::longValue ).orElse( null );
-        Integer websocketMessage = get( builder, "websocket_message", Number.class ).map( Number::intValue ).orElse( null );
+        OptionalInt port = unboxOptInt( get( builder, "port", Number.class ) );
+        OptionalInt timeout = unboxOptInt( get( builder, "timeout", Number.class ) );
+        OptionalLong maxUpload = unboxOptLong( get( builder, "max_upload", Number.class ).map( Number::longValue ) );
+        OptionalLong maxDownload = unboxOptLong( get( builder, "max_download", Number.class ).map( Number::longValue ) );
+        OptionalInt websocketMessage = unboxOptInt( get( builder, "websocket_message", Number.class ).map( Number::intValue ) );
 
         PartialOptions options = new PartialOptions(
             action,
@@ -122,6 +124,16 @@ private static <T extends Enum<T>> Optional<T> getEnum( UnmodifiableConfig confi
         return get( config, field, String.class ).map( x -> parseEnum( klass, x ) );
     }
 
+    private static OptionalLong unboxOptLong( Optional<? extends Number> value )
+    {
+        return value.map( Number::intValue ).map( OptionalLong::of ).orElse( OptionalLong.empty() );
+    }
+
+    private static OptionalInt unboxOptInt( Optional<? extends Number> value )
+    {
+        return value.map( Number::intValue ).map( OptionalInt::of ).orElse( OptionalInt.empty() );
+    }
+
     @Nullable
     private static <T extends Enum<T>> T parseEnum( Class<T> klass, String x )
     {

src/main/java/dan200/computercraft/core/apis/http/options/PartialOptions.java

@@ -5,21 +5,25 @@
  */
 package dan200.computercraft.core.apis.http.options;
 
-import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import java.util.OptionalInt;
+import java.util.OptionalLong;
 
 public final class PartialOptions
 {
-    static final PartialOptions DEFAULT = new PartialOptions( null, null, null, null, null );
+    public static final PartialOptions DEFAULT = new PartialOptions(
+        null, OptionalLong.empty(), OptionalLong.empty(), OptionalInt.empty(), OptionalInt.empty()
+    );
 
-    Action action;
-    Long maxUpload;
-    Long maxDownload;
-    Integer timeout;
-    Integer websocketMessage;
+    private final @Nullable Action action;
+    private final OptionalLong maxUpload;
+    private final OptionalLong maxDownload;
+    private final OptionalInt timeout;
+    private final OptionalInt websocketMessage;
 
-    Options options;
+    private @Nullable Options options;
 
-    PartialOptions( Action action, Long maxUpload, Long maxDownload, Integer timeout, Integer websocketMessage )
+    public PartialOptions( @Nullable Action action, OptionalLong maxUpload, OptionalLong maxDownload, OptionalInt timeout, OptionalInt websocketMessage )
     {
         this.action = action;
         this.maxUpload = maxUpload;
@@ -28,31 +32,36 @@ public final class PartialOptions
         this.websocketMessage = websocketMessage;
     }
 
-    @Nonnull
     Options toOptions()
     {
         if( options != null ) return options;
 
         return options = new Options(
             action == null ? Action.DENY : action,
-            maxUpload == null ? AddressRule.MAX_UPLOAD : maxUpload,
-            maxDownload == null ? AddressRule.MAX_DOWNLOAD : maxDownload,
-            timeout == null ? AddressRule.TIMEOUT : timeout,
-            websocketMessage == null ? AddressRule.WEBSOCKET_MESSAGE : websocketMessage
+            maxUpload.orElse( AddressRule.MAX_UPLOAD ),
+            maxDownload.orElse( AddressRule.MAX_DOWNLOAD ),
+            timeout.orElse( AddressRule.TIMEOUT ),
+            websocketMessage.orElse( AddressRule.WEBSOCKET_MESSAGE )
         );
     }
 
-    void merge( @Nonnull PartialOptions other )
+    /**
+     * Perform a left-biased union of two {@link PartialOptions}.
+     *
+     * @param other The other partial options to combine with.
+     * @return The merged options map.
+     */
+    PartialOptions merge( PartialOptions other )
     {
-        if( action == null && other.action != null ) action = other.action;
-        if( maxUpload == null && other.maxUpload != null ) maxUpload = other.maxUpload;
-        if( maxDownload == null && other.maxDownload != null ) maxDownload = other.maxDownload;
-        if( timeout == null && other.timeout != null ) timeout = other.timeout;
-        if( websocketMessage == null && other.websocketMessage != null ) websocketMessage = other.websocketMessage;
-    }
+        // Short circuit for DEFAULT. This has no effect on the outcome, but avoids an allocation.
+        if( this == DEFAULT ) return other;
 
-    PartialOptions copy()
-    {
-        return new PartialOptions( action, maxUpload, maxDownload, timeout, websocketMessage );
+        return new PartialOptions(
+            action == null && other.action != null ? other.action : action,
+            maxUpload.isPresent() ? maxUpload : other.maxUpload,
+            maxDownload.isPresent() ? maxDownload : other.maxDownload,
+            timeout.isPresent() ? timeout : other.timeout,
+            websocketMessage.isPresent() ? websocketMessage : other.websocketMessage
+        );
     }
 }

src/main/java/dan200/computercraft/core/apis/http/websocket/NoOriginWebSocketHanshakder.java

@@ -0,0 +1,35 @@
+/*
+ * This file is part of ComputerCraft - http://www.computercraft.info
+ * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission.
+ * Send enquiries to dratcliffe@gmail.com
+ */
+package dan200.computercraft.core.apis.http.websocket;
+
+import io.netty.handler.codec.http.FullHttpRequest;
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpHeaders;
+import io.netty.handler.codec.http.websocketx.WebSocketClientHandshaker13;
+import io.netty.handler.codec.http.websocketx.WebSocketVersion;
+
+import java.net.URI;
+
+/**
+ * A version of {@link WebSocketClientHandshaker13} which doesn't add the {@link HttpHeaderNames#ORIGIN} header to the
+ * original HTTP request.
+ */
+public class NoOriginWebSocketHanshakder extends WebSocketClientHandshaker13
+{
+    public NoOriginWebSocketHanshakder( URI webSocketURL, WebSocketVersion version, String subprotocol, boolean allowExtensions, HttpHeaders customHeaders, int maxFramePayloadLength )
+    {
+        super( webSocketURL, version, subprotocol, allowExtensions, customHeaders, maxFramePayloadLength );
+    }
+
+    @Override
+    protected FullHttpRequest newHandshakeRequest()
+    {
+        FullHttpRequest request = super.newHandshakeRequest();
+        HttpHeaders headers = request.headers();
+        if( !customHeaders.contains( HttpHeaderNames.ORIGIN ) ) headers.remove( HttpHeaderNames.ORIGIN );
+        return request;
+    }
+}

src/main/java/dan200/computercraft/core/apis/http/websocket/Websocket.java

@@ -26,7 +26,6 @@
 import io.netty.handler.codec.http.HttpHeaders;
 import io.netty.handler.codec.http.HttpObjectAggregator;
 import io.netty.handler.codec.http.websocketx.WebSocketClientHandshaker;
-import io.netty.handler.codec.http.websocketx.WebSocketClientHandshakerFactory;
 import io.netty.handler.codec.http.websocketx.WebSocketVersion;
 import io.netty.handler.ssl.SslContext;
 
@@ -152,7 +151,7 @@ protected void initChannel( SocketChannel ch )
                         }
 
                         String subprotocol = headers.get( HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL );
-                        WebSocketClientHandshaker handshaker = WebSocketClientHandshakerFactory.newHandshaker(
+                        WebSocketClientHandshaker handshaker = new NoOriginWebSocketHanshakder(
                             uri, WebSocketVersion.V13, subprotocol, true, headers,
                             options.websocketMessage <= 0 ? MAX_MESSAGE_SIZE : options.websocketMessage
                         );

src/main/java/dan200/computercraft/shared/computer/upload/UploadResult.java

@@ -14,8 +14,6 @@ public enum UploadResult
     CONSUMED,
     ERROR;
 
-    public static final Component SUCCESS_TITLE = new TranslatableComponent( "gui.computercraft.upload.success" );
-
     public static final Component FAILED_TITLE = new TranslatableComponent( "gui.computercraft.upload.failed" );
     public static final Component COMPUTER_OFF_MSG = new TranslatableComponent( "gui.computercraft.upload.failed.computer_off" );
     public static final Component TOO_MUCH_MSG = new TranslatableComponent( "gui.computercraft.upload.failed.too_much" );

src/main/java/dan200/computercraft/shared/util/NBTUtil.java

@@ -17,6 +17,7 @@
 import java.io.OutputStream;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -177,7 +178,7 @@ public static String getNBTHash( @Nullable CompoundTag tag )
         {
             MessageDigest digest = MessageDigest.getInstance( "MD5" );
             DataOutput output = new DataOutputStream( new DigestOutputStream( digest ) );
-            NbtIo.write( tag, output );
+            writeTag( output, "", tag );
             byte[] hash = digest.digest();
             return ENCODING.encode( hash );
         }
@@ -188,6 +189,37 @@ public static String getNBTHash( @Nullable CompoundTag tag )
         }
     }
 
+    /**
+     * An alternative version of {@link NbtIo#write(CompoundTag, DataOutput)}, which sorts keys. This
+     * should make the output slightly more deterministic.
+     *
+     * @param output The output to write to.
+     * @param name   The name of the key we're writing. Should be {@code ""} for the root node.
+     * @param tag    The tag to write.
+     * @throws IOException If the underlying stream throws.
+     * @see NbtIo#write(CompoundTag, DataOutput)
+     * @see CompoundTag#write(DataOutput)
+     */
+    private static void writeTag( DataOutput output, String name, Tag tag ) throws IOException
+    {
+        output.writeByte( tag.getId() );
+        if( tag.getId() == 0 ) return;
+        output.writeUTF( name );
+
+        if( tag instanceof CompoundTag compound )
+        {
+            String[] keys = compound.getAllKeys().toArray( new String[0] );
+            Arrays.sort( keys );
+            for( String key : keys ) writeTag( output, key, compound.get( key ) );
+
+            output.writeByte( 0 );
+        }
+        else
+        {
+            tag.write( output );
+        }
+    }
+
     private static final class DigestOutputStream extends OutputStream
     {
         private final MessageDigest digest;

src/main/resources/assets/computercraft/lang/en_us.json

@@ -116,8 +116,6 @@
     "gui.computercraft.tooltip.turn_off.key": "Hold Ctrl+S",
     "gui.computercraft.tooltip.terminate": "Stop the currently running code",
     "gui.computercraft.tooltip.terminate.key": "Hold Ctrl+T",
-    "gui.computercraft.upload.success": "Upload Succeeded",
-    "gui.computercraft.upload.success.msg": "%d files uploaded.",
     "gui.computercraft.upload.failed": "Upload Failed",
     "gui.computercraft.upload.failed.computer_off": "You must turn the computer on before uploading files.",
     "gui.computercraft.upload.failed.too_much": "Your files are too large to be uploaded.",

src/main/resources/data/computercraft/lua/rom/programs/edit.lua

@@ -55,19 +55,22 @@ term.redirect(current)
 term.setTextColor(term.isColour() and colours.yellow or colours.white)
 term.setBackgroundColor(colours.black)
 term.setCursorBlink(false)
-local _, y = term.getCursorPos()
-local _, h = term.getSize()
 if not ok then
     printError(err)
 end
-if ok and y >= h then
-    term.scroll(1)
+
+local message = "Press any key to continue."
+if ok then message = "Program finished. " .. message end
+local _, y = term.getCursorPos()
+local w, h = term.getSize()
+local wrapped = require("cc.strings").wrap(message, w)
+
+local start_y = h - #wrapped + 1
+if y >= start_y then term.scroll(y - start_y + 1) end
+for i = 1, #wrapped do
+    term.setCursorPos(1, start_y + i - 1)
+    term.write(wrapped[i])
 end
-term.setCursorPos(1, h)
-if ok then
-    write("Program finished. ")
-end
-write("Press any key to continue")
 os.pullEvent('key')
 ]]
 

src/test/java/dan200/computercraft/core/apis/http/options/AddressRuleTest.java

@@ -12,6 +12,7 @@
 
 import java.net.InetSocketAddress;
 import java.util.Collections;
+import java.util.OptionalInt;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
@@ -21,8 +22,8 @@ public class AddressRuleTest
     public void matchesPort()
     {
         Iterable<AddressRule> rules = Collections.singletonList( AddressRule.parse(
-            "127.0.0.1", 8080,
-            new PartialOptions( Action.ALLOW, null, null, null, null )
+            "127.0.0.1", OptionalInt.of( 8080 ),
+            Action.ALLOW.toPartial()
         ) );
 
         assertEquals( apply( rules, "localhost", 8080 ).action, Action.ALLOW );

src/test/resources/test-rom/spec/apis/os_spec.lua

@@ -61,9 +61,9 @@ describe("The os library", function()
 
             -- TODO: Java 16 apparently no longer treats TextStyle.FULL as full and will render Sun instead of Sunday.
             exp_code("%a", "Sun")
-            -- exp_code("%A", "Sunday")
+            exp_code("%A", "Sunday")
             exp_code("%b", "Oct")
-            -- exp_code("%B", "October")
+            exp_code("%B", "October")
             exp_code("%c", "Sun Oct  1 23:12:17 2000")
             exp_code("%C", "20")
             exp_code("%d", "01")

src/testFixtures/kotlin/dan200/computercraft/test/core/computer/KotlinLuaMachine.kt

@@ -5,11 +5,9 @@
 import dan200.computercraft.core.lua.ILuaMachine
 import dan200.computercraft.core.lua.MachineEnvironment
 import dan200.computercraft.core.lua.MachineResult
-import kotlinx.coroutines.CoroutineName
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.launch
+import kotlinx.coroutines.*
 import java.io.InputStream
+import kotlin.coroutines.CoroutineContext
 
 /**
  * An [ILuaMachine] which runs Kotlin functions instead.
@@ -26,7 +24,7 @@ override fun handleEvent(eventName: String?, arguments: Array<out Any>?): Machin
             queueEvent(eventName, arguments)
         } else {
             val task = getTask()
-            if (task != null) CoroutineScope(Dispatchers.Unconfined + CoroutineName("Computer")).launch { task() }
+            if (task != null) CoroutineScope(NeverDispatcher() + CoroutineName("Computer")).launch { task() }
         }
 
         return MachineResult.OK
@@ -38,4 +36,20 @@ override fun printExecutionState(out: StringBuilder) {}
      * Get the next task to execute on this computer.
      */
     protected abstract fun getTask(): (suspend KotlinLuaMachine.() -> Unit)?
+
+    /**
+     * A [CoroutineDispatcher] which only allows resuming from the computer thread. In practice, this means the only
+     * way to yield is with [pullEvent].
+     */
+    private class NeverDispatcher : CoroutineDispatcher() {
+        private val expectedGroup = Thread.currentThread().threadGroup
+
+        override fun dispatch(context: CoroutineContext, block: Runnable) {
+            if (Thread.currentThread().threadGroup != expectedGroup) {
+                throw UnsupportedOperationException("Cannot perform arbitrary yields")
+            }
+
+            block.run()
+        }
+    }
 }

src/testFixtures/kotlin/dan200/computercraft/test/core/computer/LuaTaskContext.kt

@@ -4,9 +4,11 @@
 import dan200.computercraft.api.lua.ILuaContext
 import dan200.computercraft.api.lua.MethodResult
 import dan200.computercraft.api.lua.ObjectArguments
+import dan200.computercraft.core.apis.OSAPI
 import dan200.computercraft.core.apis.PeripheralAPI
 import kotlinx.coroutines.CancellableContinuation
 import kotlinx.coroutines.suspendCancellableCoroutine
+import kotlin.time.Duration
 
 /**
  * The context for tasks which consume Lua objects.
@@ -40,6 +42,19 @@ suspend fun pullEvent(event: String? = null): Array<out Any?>
     /** Call a peripheral method. */
     suspend fun LuaTaskContext.callPeripheral(name: String, method: String, vararg args: Any?): Array<out Any?>? =
         getApi<PeripheralAPI>().call(context, ObjectArguments(name, method, *args)).await()
+
+    /**
+     * Sleep for the given duration. This uses the internal computer clock, so won't be accurate.
+     */
+    suspend fun LuaTaskContext.sleep(duration: Duration) {
+        val timer = getApi<OSAPI>().startTimer(duration.inWholeMilliseconds / 1000.0)
+        while (true) {
+            val event = pullEvent("timer")
+            if (event[0] == "timer" && event[1] is Number && (event[1] as Number).toInt() == timer) {
+                return
+            }
+        }
+    }
 }
 
 /** Get a registered API. */

src/testMod/kotlin/dan200/computercraft/gametest/Modem_Test.kt

@@ -13,7 +13,6 @@
 import dan200.computercraft.test.core.assertArrayEquals
 import dan200.computercraft.test.core.computer.LuaTaskContext
 import dan200.computercraft.test.core.computer.getApi
-import kotlinx.coroutines.delay
 import net.minecraft.core.BlockPos
 import net.minecraft.gametest.framework.GameTest
 import net.minecraft.gametest.framework.GameTestHelper
@@ -60,7 +59,7 @@ fun Transmits_messages(context: GameTestHelper) = context.sequence {
             val modem = findPeripheral("modem") ?: throw IllegalStateException("Cannot find modem")
             while (true) {
                 callPeripheral(modem, "transmit", 12, 34, "Hello")
-                delay(50.milliseconds)
+                sleep(50.milliseconds)
             }
         }
         thenOnComputer("receive") {