Browse Source

Do not close unowned socket in Http.customRequest (#12069)

* Add test for passing socket to Http.customRequest

If we create the socket, we should also close it ourselves.

* Do not close unowned socket in Http.customRequest

If the socket doesn't belong to us, it is not safe to close it.

For example, on android which enables fdsan (file descriptor sanitizer),
this can cause a hard crash if the socket comes from another thread.
tobil4sk 3 months ago
parent
commit
cbbb319ec7
2 changed files with 40 additions and 4 deletions
  1. 8 4
      std/sys/Http.hx
  2. 32 0
      tests/unit/src/unit/TestHttps.hx

+ 8 - 4
std/sys/Http.hx

@@ -98,6 +98,7 @@ class Http extends haxe.http.HttpBase {
 			return;
 		}
 		var secure = (url_regexp.matched(1) == "https://");
+		var ownsSocket = false;
 		if (sock == null) {
 			if (secure) {
 				#if php
@@ -117,6 +118,7 @@ class Http extends haxe.http.HttpBase {
 				sock = new Socket();
 			}
 			sock.setTimeout(cnxTimeout);
+			ownsSocket = true;
 		}
 		var host = url_regexp.matched(2);
 		var portString = url_regexp.matched(3);
@@ -246,11 +248,13 @@ class Http extends haxe.http.HttpBase {
 			else
 				writeBody(b, null, 0, null, sock);
 			readHttpResponse(api, sock);
-			sock.close();
+			if (ownsSocket)
+				sock.close();
 		} catch (e:Dynamic) {
-			try
-				sock.close()
-			catch (e:Dynamic) {};
+			if (ownsSocket)
+				try
+					sock.close()
+				catch (e:Dynamic) {};
 			onError(Std.string(e));
 		}
 	}

+ 32 - 0
tests/unit/src/unit/TestHttps.hx

@@ -1,5 +1,6 @@
 package unit;
 
+import utest.Assert;
 import utest.Async;
 
 class TestHttps extends Test {
@@ -59,4 +60,35 @@ class TestHttps extends Test {
 		}
 		req.request();
 	});
+
+	#if sys
+	@:timeout(3000)
+	public function testCustomRequestWithSocket(async:Async) run(async, () -> {
+		final url = 'http://build.haxe.org/builds/haxe/linux64/haxe_latest.tar.gz';
+
+		final http = new haxe.Http(url);
+		final socket = new sys.net.Socket();
+		socket.setTimeout(10);
+		final output = new haxe.io.BytesOutput();
+		http.customRequest(false, output, socket);
+
+		try {
+			// our socket shouldn't be closed until we close it,
+			// so this should work
+			Assert.notNull(socket.host());
+			socket.setTimeout(10);
+			noAssert();
+		} catch (e) {
+			assert('socket should be unclosed, but got error: $e');
+		}
+
+		try {
+			socket.close();
+			noAssert();
+		} catch (e) {
+			assert("Failed to close socket");
+		}
+		async.done();
+	});
+	#end
 }