diff --git a/src/main/resources/assets/opencomputers/loot/openos/boot/01_process.lua b/src/main/resources/assets/opencomputers/loot/openos/boot/01_process.lua index 4fc9428eb4..046852b059 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/boot/01_process.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/boot/01_process.lua @@ -52,7 +52,9 @@ end _coroutine.wrap = function(f) local thread = coroutine.create(f) return function(...) - return select(2, coroutine.resume(thread, ...)) + local result_pack = table.pack(coroutine.resume(thread, ...)) + if not result_pack[1] then error(result_pack[2], 2) end + return table.unpack(result_pack, 2, result_pack.n) end end diff --git a/src/main/resources/assets/opencomputers/loot/openos/lib/thread.lua b/src/main/resources/assets/opencomputers/loot/openos/lib/thread.lua index bdb826899d..f019abe1d9 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/lib/thread.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/lib/thread.lua @@ -265,6 +265,7 @@ function thread.create(fp, ...) local old_status = t:status() mt.__status = "dead" process.removeHandle(t, mt.attached) + process.removeHandle(mt.process, t) if old_status ~= "dead" then event.push("thread_exit") end diff --git a/src/main/scala/li/cil/oc/integration/opencomputers/DriverUpgradeLeash.scala b/src/main/scala/li/cil/oc/integration/opencomputers/DriverUpgradeLeash.scala index 9e4fd3ba90..515050d112 100644 --- a/src/main/scala/li/cil/oc/integration/opencomputers/DriverUpgradeLeash.scala +++ b/src/main/scala/li/cil/oc/integration/opencomputers/DriverUpgradeLeash.scala @@ -4,6 +4,7 @@ import li.cil.oc.Constants import li.cil.oc.api import li.cil.oc.api.driver.EnvironmentProvider import li.cil.oc.api.driver.item.HostAware +import li.cil.oc.api.internal.Drone import li.cil.oc.api.network.EnvironmentHost import li.cil.oc.common.Slot import li.cil.oc.common.Tier @@ -18,7 +19,7 @@ object DriverUpgradeLeash extends Item with HostAware { override def createEnvironment(stack: ItemStack, host: EnvironmentHost) = if (host.getEnvironmentLevel != null && host.getEnvironmentLevel.isClientSide) null else host match { - case entity: Entity => new component.UpgradeLeash(entity) + case entity: Entity with Drone => new component.UpgradeLeash(entity) case _ => null } diff --git a/src/main/scala/li/cil/oc/server/component/InternetCard.scala b/src/main/scala/li/cil/oc/server/component/InternetCard.scala index dcb3cd2fd1..2f12c0a28e 100644 --- a/src/main/scala/li/cil/oc/server/component/InternetCard.scala +++ b/src/main/scala/li/cil/oc/server/component/InternetCard.scala @@ -63,7 +63,7 @@ class InternetCard extends AbstractManagedEnvironment with DeviceInfo { @Callback(direct = true, doc = """function():boolean -- Returns whether HTTP requests can be made (config setting).""") def isHttpEnabled(context: Context, args: Arguments): Array[AnyRef] = result(Settings.get.httpEnabled) - @Callback(doc = """function(url:string[, postData:string[, headers:table[, method:string]]]):userdata -- Starts an HTTP request. If this returns true, further results will be pushed using `http_response` signals.""") + @Callback(doc = """function(url:string[, postData:string[, headers:table[, method:string[, allowErrorBody:boolean]]]]):userdata -- Starts an HTTP request. If allowErrorBody is true, HTTP error responses will return their body instead of throwing an exception.""") def request(context: Context, args: Arguments): Array[AnyRef] = this.synchronized { checkOwner(context) val address = args.checkString(0) @@ -85,7 +85,8 @@ class InternetCard extends AbstractManagedEnvironment with DeviceInfo { return result((), "http request headers are unavailable") } val method = if (args.isString(3)) Option(args.checkString(3)) else None - val request = new InternetCard.HTTPRequest(this, checkAddress(address), post, headers, method) + val allowErrorBody = args.optBoolean(4, false) + val request = new InternetCard.HTTPRequest(this, checkAddress(address), post, headers, method, allowErrorBody) connections += request result(request) } @@ -331,7 +332,9 @@ object InternetCard { private def checkConnected() = { if (owner.isEmpty) throw new IOException("connection lost") try { - if (isAddressResolved) channel.finishConnect() + if (isAddressResolved) { + channel.finishConnect() + } else if (address.isCancelled) { // I don't think this can ever happen, Justin Case. channel.close() @@ -343,14 +346,15 @@ object InternetCard { case e: ExecutionException => throw e.getCause } isAddressResolved = true - false + // After address resolution, immediately attempt connection. + channel.finishConnect() } else false } catch { case t: Throwable => close() - false + throw t } } @@ -426,11 +430,31 @@ object InternetCard { } } + private[component] def responseStream(http: HttpURLConnection, allowErrorBody: Boolean): InputStream = { + val responseCode = http.getResponseCode + if (responseCode >= 200 && responseCode < 300) { + // Successful responses always use getInputStream(). + http.getInputStream + } + else if (allowErrorBody) { + // Error responses may expose their body when explicitly requested. + Option(http.getErrorStream).getOrElse(new java.io.ByteArrayInputStream(Array.empty[Byte])) + } + else { + // Preserve the historical behavior for existing callers. + http.getInputStream + } + } + class HTTPRequest extends AbstractValue with Closable { - def this(owner: InternetCard, url: URL, post: Option[String], headers: Map[String, String], method: Option[String]) = { + def this(owner: InternetCard, url: URL, post: Option[String], headers: Map[String, String], method: Option[String], allowErrorBody: Boolean) = { this() this.owner = Some(owner) - this.stream = threadPool.submit(new RequestSender(url, post, headers, method)) + this.stream = threadPool.submit(new RequestSender(url, post, headers, method, allowErrorBody)) + } + + def this(owner: InternetCard, url: URL, post: Option[String], headers: Map[String, String], method: Option[String]) = { + this(owner, url, post, headers, method, false) } private var owner: Option[InternetCard] = None @@ -529,7 +553,7 @@ object InternetCard { } // This one doesn't (see comment in TCP socket), but I like to keep it consistent. - private class RequestSender(val url: URL, val post: Option[String], val headers: Map[String, String], val method: Option[String]) extends Callable[InputStream] { + private class RequestSender(val url: URL, val post: Option[String], val headers: Map[String, String], val method: Option[String], val allowErrorBody: Boolean) extends Callable[InputStream] { override def call() = try { checkLists(InetAddress.getByName(url.getHost), url.getHost) val proxy = ServerLifecycleHooks.getCurrentServer.proxy @@ -564,9 +588,7 @@ object InternetCard { response = Some((http.getResponseCode, http.getResponseMessage, http.getHeaderFields)) } - // TODO: This should allow accessing getErrorStream() for reading unsuccessful HTTP responses' output, - // but this would be a breaking change for existing OC code. - http.getInputStream + responseStream(http, allowErrorBody) } catch { case t: Throwable => diff --git a/src/main/scala/li/cil/oc/server/component/Trade.scala b/src/main/scala/li/cil/oc/server/component/Trade.scala index bdaa50506f..f99f70d251 100644 --- a/src/main/scala/li/cil/oc/server/component/Trade.scala +++ b/src/main/scala/li/cil/oc/server/component/Trade.scala @@ -81,7 +81,7 @@ class Trade(val info: TradeInfo) extends AbstractValue { if (!hasRoomForRecipe(inventory, recipe)) { result(false, "not enough inventory space to trade") } else { - if (completeTrade(inventory, recipe, exact = true) || completeTrade(inventory, recipe, exact = false)) { + if (completeTrade(inventory, recipe, exact = true)) { result(true) } else { result(false, "not enough items to trade") diff --git a/src/main/scala/li/cil/oc/server/component/UpgradeLeash.scala b/src/main/scala/li/cil/oc/server/component/UpgradeLeash.scala index 2b7591654e..785bcd634b 100644 --- a/src/main/scala/li/cil/oc/server/component/UpgradeLeash.scala +++ b/src/main/scala/li/cil/oc/server/component/UpgradeLeash.scala @@ -8,6 +8,7 @@ import li.cil.oc.api.driver.DeviceInfo.DeviceClass import li.cil.oc.OpenComputers import li.cil.oc.api.Network import li.cil.oc.api.driver.DeviceInfo +import li.cil.oc.api.internal import li.cil.oc.api.machine.Arguments import li.cil.oc.api.machine.Callback import li.cil.oc.api.machine.Context @@ -25,6 +26,8 @@ import net.minecraft.core.HolderLookup import net.minecraft.core.component.DataComponentHolder import net.minecraft.world.entity.Entity import net.minecraft.world.entity.Mob +import net.minecraft.world.item.ItemStack +import net.minecraft.world.item.Items import net.minecraft.nbt.CompoundTag import net.minecraft.nbt.StringTag @@ -34,7 +37,7 @@ import scala.collection.mutable import net.minecraft.nbt.Tag import net.neoforged.neoforge.common.MutableDataComponentHolder -class UpgradeLeash(val host: Entity) extends AbstractManagedEnvironment with traits.LevelAware with DeviceInfo { +class UpgradeLeash(val host: Entity with internal.Drone) extends AbstractManagedEnvironment with traits.LevelAware with DeviceInfo { override val node = Network.newNode(this, Visibility.Network). withComponent("leash"). create() @@ -53,7 +56,7 @@ class UpgradeLeash(val host: Entity) extends AbstractManagedEnvironment with tra val leashedEntities = mutable.Set.empty[UUID] - override def position = BlockPosition(host) + override def position = BlockPosition(host.asInstanceOf[Entity]) @Callback(doc = """function(side:number):boolean -- Tries to put an entity on the specified side of the device onto a leash.""") def leash(context: Context, args: Arguments): Array[AnyRef] = { @@ -64,11 +67,16 @@ class UpgradeLeash(val host: Entity) extends AbstractManagedEnvironment with tra val bounds = nearBounds.minmax(farBounds) entitiesInBounds[Mob](classOf[Mob], bounds).find(_.canBeLeashed()) match { case Some(entity) => - entity.setLeashedTo(host, false) - leashedEntities += entity.getUUID - context.pause(0.1) - result(true) - case _ => result((), "no unleashed entity") + if (shrinkLeash()) { + entity.setLeashedTo(host, false) + leashedEntities += entity.getUUID + context.pause(0.1) + result(true) + } + else { + result(false, "no lead in inventory") + } + case _ => result(false, "no unleashed entity") } } @@ -88,12 +96,51 @@ class UpgradeLeash(val host: Entity) extends AbstractManagedEnvironment with tra private def unleashAll(): Unit = { entitiesInBounds(classOf[Mob], position.bounds.inflate(5, 5, 5)).foreach(entity => { if (leashedEntities.contains(entity.getUUID) && entity.getLeashHolder == host) { - entity.dropLeash(true, false) + if (returnLeash()) { + entity.dropLeash(true, false) + leashedEntities -= entity.getUUID + } } }) leashedEntities.clear() } + private def shrinkLeash(): Boolean = { + val inventory = host.mainInventory() + for (index <- 0 until inventory.getContainerSize) { + val stack = inventory.getItem(index) + if (!stack.isEmpty && stack.getItem == Items.LEAD) { + stack.shrink(1) + if (stack.isEmpty) { + inventory.setItem(index, ItemStack.EMPTY) + } + return true + } + } + false + } + + private def returnLeash(): Boolean = { + val inventory = host.mainInventory() + + for (index <- 0 until inventory.getContainerSize) { + val stack = inventory.getItem(index) + if (!stack.isEmpty && stack.getItem == Items.LEAD && stack.getCount < stack.getMaxStackSize) { + stack.grow(1) + return true + } + } + + for (index <- 0 until inventory.getContainerSize) { + if (inventory.getItem(index).isEmpty) { + inventory.setItem(index, new ItemStack(Items.LEAD)) + return true + } + } + + false + } + override def loadData(holder: DataComponentHolder): Unit = { super.loadData(holder) for(entities <- holder.getComponent(OCComponents.LEASHED_ENTITIES)) diff --git a/src/main/scala/li/cil/oc/server/fs/FileSystem.scala b/src/main/scala/li/cil/oc/server/fs/FileSystem.scala index fb5e30313b..0f317d834b 100755 --- a/src/main/scala/li/cil/oc/server/fs/FileSystem.scala +++ b/src/main/scala/li/cil/oc/server/fs/FileSystem.scala @@ -179,14 +179,16 @@ object FileSystem extends api.detail.FileSystemAPI { abstract class ItemLabel(val stack: ItemStack) extends Label - class ReadOnlyLabel(val label: String) extends Label { + class ReadOnlyLabel(private var label: String) extends Label { def setLabel(value: String) = throw new IllegalArgumentException("label is read only") def getLabel(provider: HolderLookup.Provider): String = label - private final val LabelTag = Settings.namespace + "fs.label" - - override def loadData(holder: DataComponentHolder): Unit = {} + override def loadData(holder: DataComponentHolder): Unit = { + for (value <- holder.getComponent(OCComponents.LABEL)) { + label = value + } + } override def saveData(holder: MutableDataComponentHolder): Unit = { if(label != null) { diff --git a/src/main/scala/li/cil/oc/server/machine/Machine.scala b/src/main/scala/li/cil/oc/server/machine/Machine.scala index 65b3ec20ba..f036169ccd 100644 --- a/src/main/scala/li/cil/oc/server/machine/Machine.scala +++ b/src/main/scala/li/cil/oc/server/machine/Machine.scala @@ -469,7 +469,7 @@ class Machine(val host: MachineHost) extends AbstractManagedEnvironment with mac def isRunning(context: Context, args: Arguments): Array[AnyRef] = result(isRunning) - @Callback(doc = """function([frequency:string or number[, duration:number]]) -- Plays a tone, useful to alert users via audible feedback.""") + @Callback(doc = """function([frequency:string or number[, duration:number[, async:boolean]]]) -- Plays a tone, useful to alert users via audible feedback.""") def beep(context: Context, args: Arguments): Array[AnyRef] = { if (args.count == 1 && args.isString(0)) { beep(args.checkString(0)) @@ -480,7 +480,10 @@ class Machine(val host: MachineHost) extends AbstractManagedEnvironment with mac } val duration = args.optDouble(1, 0.1) val durationInMilliseconds = math.max(50, math.min(5000, (duration * 1000).toInt)) - context.pause(durationInMilliseconds / 1000.0) + val async = args.optBoolean(2, false) + if (!async) { + context.pause(durationInMilliseconds / 1000.0) + } beep(frequency.toShort, durationInMilliseconds.toShort) } null diff --git a/src/test/scala/InternetCardHttpTest.scala b/src/test/scala/InternetCardHttpTest.scala new file mode 100644 index 0000000000..f85f47d9ef --- /dev/null +++ b/src/test/scala/InternetCardHttpTest.scala @@ -0,0 +1,87 @@ +package li.cil.oc.server.component + +import org.junit.runner.RunWith +import org.scalatest.funspec.AnyFunSpec +import org.scalatest.matchers.should.Matchers +import org.scalatestplus.junit.JUnitRunner + +import java.io.{ByteArrayInputStream, IOException, InputStream} +import java.net.{HttpURLConnection, URL} +import java.nio.charset.StandardCharsets + +@RunWith(classOf[JUnitRunner]) +class InternetCardHttpTest extends AnyFunSpec with Matchers { + describe("Internet Card HTTP response handling") { + it("uses the input stream for successful responses") { + val connection = new StubHttpURLConnection(200, "ok", Some("ignored")) + + read(InternetCard.responseStream(connection, allowErrorBody = false)) shouldBe "ok" + connection.inputStreamCalls shouldBe 1 + connection.errorStreamCalls shouldBe 0 + } + + it("preserves the legacy exception behavior for error responses") { + val connection = new StubHttpURLConnection(404, "unused", Some("not found")) + + intercept[IOException] { + InternetCard.responseStream(connection, allowErrorBody = false) + } + connection.inputStreamCalls shouldBe 1 + connection.errorStreamCalls shouldBe 0 + } + + it("returns an error response body when explicitly allowed") { + val connection = new StubHttpURLConnection(404, "unused", Some("not found")) + + read(InternetCard.responseStream(connection, allowErrorBody = true)) shouldBe "not found" + connection.inputStreamCalls shouldBe 0 + connection.errorStreamCalls shouldBe 1 + } + + it("returns an empty stream when an allowed error has no body") { + val connection = new StubHttpURLConnection(503, "unused", None) + + read(InternetCard.responseStream(connection, allowErrorBody = true)) shouldBe "" + connection.inputStreamCalls shouldBe 0 + connection.errorStreamCalls shouldBe 1 + } + + it("does not use the error stream for successful responses even when enabled") { + val connection = new StubHttpURLConnection(204, "", Some("must not be read")) + + read(InternetCard.responseStream(connection, allowErrorBody = true)) shouldBe "" + connection.inputStreamCalls shouldBe 1 + connection.errorStreamCalls shouldBe 0 + } + } + + private def read(stream: InputStream): String = { + try new String(stream.readAllBytes(), StandardCharsets.UTF_8) + finally stream.close() + } + + private class StubHttpURLConnection(responseCode: Int, inputBody: String, errorBody: Option[String]) + extends HttpURLConnection(new URL("http://example.invalid")) { + var inputStreamCalls = 0 + var errorStreamCalls = 0 + + override def connect(): Unit = () + + override def disconnect(): Unit = () + + override def usingProxy(): Boolean = false + + override def getResponseCode: Int = responseCode + + override def getInputStream: InputStream = { + inputStreamCalls += 1 + if (responseCode >= 300) throw new IOException("HTTP error") + new ByteArrayInputStream(inputBody.getBytes(StandardCharsets.UTF_8)) + } + + override def getErrorStream: InputStream = { + errorStreamCalls += 1 + errorBody.map(body => new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8))).orNull + } + } +}