[Asterisk-code-review] http: Send headers and body in one write. (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/4551 )

Change subject: http: Send headers and body in one write.
......................................................................


http: Send headers and body in one write.

This is a semi-regression caused by the iostreams change. Prior to
iostreams, HTTP headers were written to a FILE handle using fprintf.
Then the body was written using a call to fwrite(). Because of internal
buffering, the result was that the HTTP headers and body would be sent
out in a single write to the socket.

With the change to iostreams, the HTTP headers are written using
ast_iostream_printf(), which under the hood calls write(). The HTTP body
calls ast_iostream_write(), which also calls write() under the hood.
This results in two separate writes to the socket.

Most HTTP client libraries out there will handle this change just fine.
However, a few of our testsuite tests started failing because of the
change. As a result, in order to reduce frustration for users, this
change alters the HTTP code to write the headers and body in a single
write operation.

ASTERISK-26629 #close
Reported by Joshua Colp

Change-Id: Idc2d2fb3d9b3db14b8631a1e302244fa18b0e518
---
M main/http.c
1 file changed, 12 insertions(+), 17 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Matthew Fredrickson: Looks good to me, approved
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/main/http.c b/main/http.c
index 9aff4d1..5f57b1e 100644
--- a/main/http.c
+++ b/main/http.c
@@ -454,6 +454,7 @@
 	int content_length = 0;
 	int close_connection;
 	struct ast_str *server_header_field = ast_str_create(MAX_SERVER_NAME_LENGTH);
+	int send_content;
 
 	if (!ser || !server_header_field) {
 		/* The connection is not open. */
@@ -504,6 +505,8 @@
 		lseek(fd, 0, SEEK_SET);
 	}
 
+	send_content = method != AST_HTTP_HEAD || status_code >= 400;
+
 	/* send http header */
 	ast_iostream_printf(ser->stream,
 		"HTTP/1.1 %d %s\r\n"
@@ -513,33 +516,25 @@
 		"%s"
 		"%s"
 		"Content-Length: %d\r\n"
-		"\r\n",
+		"\r\n"
+		"%s",
 		status_code, status_title ? status_title : "OK",
 		ast_str_buffer(server_header_field),
 		timebuf,
 		close_connection ? "Connection: close\r\n" : "",
 		static_content ? "" : "Cache-Control: no-cache, no-store\r\n",
 		http_header ? ast_str_buffer(http_header) : "",
-		content_length
+		content_length,
+		send_content && out && ast_str_strlen(out) ? ast_str_buffer(out) : ""
 		);
 
 	/* send content */
-	if (method != AST_HTTP_HEAD || status_code >= 400) {
-		if (out && ast_str_strlen(out)) {
-			len = ast_str_strlen(out);
-			if (ast_iostream_write(ser->stream, ast_str_buffer(out), len) != len) {
-				ast_log(LOG_ERROR, "fwrite() failed: %s\n", strerror(errno));
+	if (send_content && fd) {
+		while ((len = read(fd, buf, sizeof(buf))) > 0) {
+			if (ast_iostream_write(ser->stream, buf, len) != len) {
+				ast_log(LOG_WARNING, "fwrite() failed: %s\n", strerror(errno));
 				close_connection = 1;
-			}
-		}
-
-		if (fd) {
-			while ((len = read(fd, buf, sizeof(buf))) > 0) {
-				if (ast_iostream_write(ser->stream, buf, len) != len) {
-					ast_log(LOG_WARNING, "fwrite() failed: %s\n", strerror(errno));
-					close_connection = 1;
-					break;
-				}
+				break;
 			}
 		}
 	}

-- 
To view, visit https://gerrit.asterisk.org/4551
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Idc2d2fb3d9b3db14b8631a1e302244fa18b0e518
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>