From e342762509606446301b034f0922566c2f055bc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=92=D0=B0=D1=88=D0=B5=20=D0=98=D0=BC=D1=8F?= Date: Sat, 7 Jun 2025 21:20:54 +0700 Subject: [PATCH 1/2] Fix HTTP error response handling in Internet API (#3663) Previously, HTTP error responses (4xx, 5xx) would throw exceptions instead of allowing access to response information. This made it impossible to handle API errors gracefully in Lua scripts. Changes: - Capture response code/message/headers before attempting to read stream - Use getErrorStream() for HTTP error responses instead of getInputStream() - Provide empty stream fallback when getErrorStream() returns null - Preserve existing behavior for successful responses (2xx) This allows scripts to check response codes and read error response bodies using the existing response() and read() methods. Fixes #3663 --- .../oc/server/component/InternetCard.scala | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) 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 a13c8912d5..77b29ccd77 100644 --- a/src/main/scala/li/cil/oc/server/component/InternetCard.scala +++ b/src/main/scala/li/cil/oc/server/component/InternetCard.scala @@ -525,21 +525,25 @@ object InternetCard { out.close() } - // Finish the connection. Call getInputStream a second time below to re-throw any exception. - // This avoids getResponseCode() waiting for the connection to end in the synchronized block. - try { - http.getInputStream - } catch { - case _: Exception => - } - HTTPRequest.this.synchronized { 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 + // For successful responses (2xx), use getInputStream() + // For error responses (4xx, 5xx), use getErrorStream() if available, otherwise return empty stream + val responseCode = http.getResponseCode + if (responseCode >= 200 && responseCode < 300) { + http.getInputStream + } else { + // For HTTP error responses, try to get the error stream + val errorStream = http.getErrorStream + if (errorStream != null) { + errorStream + } else { + // If no error stream is available, return an empty stream + new java.io.ByteArrayInputStream(Array.empty[Byte]) + } + } } catch { case t: Throwable => From c1152c57c303d7c8be10dee90190f5467d14e95b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=92=D0=B0=D1=88=D0=B5=20=D0=98=D0=BC=D1=8F?= Date: Mon, 16 Jun 2025 00:38:57 +0700 Subject: [PATCH 2/2] Add allowErrorBody parameter to HTTP requests for backward-compatible error handling --- .../oc/server/component/InternetCard.scala | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) 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 77b29ccd77..4dfeb388c0 100644 --- a/src/main/scala/li/cil/oc/server/component/InternetCard.scala +++ b/src/main/scala/li/cil/oc/server/component/InternetCard.scala @@ -60,7 +60,7 @@ class InternetCard extends prefab.ManagedEnvironment 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) @@ -82,7 +82,8 @@ class InternetCard extends prefab.ManagedEnvironment with DeviceInfo { return result(Unit, "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) } @@ -404,10 +405,14 @@ object InternetCard { } 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 @@ -506,7 +511,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 = Option(MinecraftServer.getServer.getServerProxy).getOrElse(java.net.Proxy.NO_PROXY) @@ -529,13 +534,12 @@ object InternetCard { response = Some((http.getResponseCode, http.getResponseMessage, http.getHeaderFields)) } - // For successful responses (2xx), use getInputStream() - // For error responses (4xx, 5xx), use getErrorStream() if available, otherwise return empty stream val responseCode = http.getResponseCode if (responseCode >= 200 && responseCode < 300) { + // Successful responses (2xx) - always use getInputStream() http.getInputStream - } else { - // For HTTP error responses, try to get the error stream + } else if (allowErrorBody) { + // HTTP error responses (4xx, 5xx) with allowErrorBody=true - try to get error stream val errorStream = http.getErrorStream if (errorStream != null) { errorStream @@ -543,6 +547,9 @@ object InternetCard { // If no error stream is available, return an empty stream new java.io.ByteArrayInputStream(Array.empty[Byte]) } + } else { + // HTTP error responses (4xx, 5xx) with allowErrorBody=false - throw exception (old behavior) + http.getInputStream // This will throw an exception for HTTP error responses } } catch {