From b69b8aaaf9e2ae3560f369a061f28385c4d87452 Mon Sep 17 00:00:00 2001 From: Jocelyn Fiat Date: Wed, 16 Sep 2015 22:51:58 +0200 Subject: [PATCH] Added first support for persistent connection in NET http client implementation. Various improvement related to eventual errors. --- .../network/http_client/http_client-safe.ecf | 27 +-- library/network/http_client/http_client.ecf | 26 +-- .../http_client/src/http_client_response.e | 29 +++ .../http_client/src/http_client_session.e | 24 +++ .../net_http_client_connection.e | 52 ++++++ .../src/spec/net/net_http_client_request.e | 171 +++++++++++++----- .../src/spec/net/net_http_client_session.e | 27 +++ .../http_client/tests/test_net_http_client.e | 44 +++++ 8 files changed, 326 insertions(+), 74 deletions(-) create mode 100644 library/network/http_client/src/spec/net/implementation/net_http_client_connection.e diff --git a/library/network/http_client/http_client-safe.ecf b/library/network/http_client/http_client-safe.ecf index e00d349c..fc4bd799 100644 --- a/library/network/http_client/http_client-safe.ecf +++ b/library/network/http_client/http_client-safe.ecf @@ -7,7 +7,7 @@ /EIFGENs$ /.svn$ - @@ -15,7 +15,6 @@ - @@ -30,51 +29,55 @@ - - - + + - + + - + - + - + - + - + - + diff --git a/library/network/http_client/http_client.ecf b/library/network/http_client/http_client.ecf index 3ab591ce..984b686e 100644 --- a/library/network/http_client/http_client.ecf +++ b/library/network/http_client/http_client.ecf @@ -15,7 +15,6 @@ - @@ -30,57 +29,60 @@ - - - + + - + + - + - + - + - + - + - + - diff --git a/library/network/http_client/src/http_client_response.e b/library/network/http_client/src/http_client_response.e index fe9c7cd4..ed297e20 100644 --- a/library/network/http_client/src/http_client_response.e +++ b/library/network/http_client/src/http_client_response.e @@ -217,6 +217,22 @@ feature -- Access end end +feature -- Status report + + is_http_1_0: BOOLEAN + -- Is response using HTTP/1.0 protocole? + --| Note: it is relevant once the raw header are set. + do + Result := attached http_version as v and then v.same_string ("1.0") + end + + is_http_1_1: BOOLEAN + -- Is response using HTTP/1.1 protocole? + --| Note: it is relevant once the raw header are set. + do + Result := attached http_version as v and then v.same_string ("1.1") + end + feature -- Change set_http_version (v: like http_version) @@ -345,10 +361,23 @@ feature -- Change set_raw_header (h: READABLE_STRING_8) -- Set http header `raw_header' to `h' + local + i: INTEGER + s: STRING_8 do raw_header := h --| Reset internal headers internal_headers := Void + + --| Set status line, right away. + i := h.index_of ('%N', 1) + if i > 0 then + s := h.substring (1, i - 1) + if s.starts_with ("HTTP/") then + s.right_adjust + set_status_line (s) + end + end end add_redirection (s: detachable READABLE_STRING_8; h: READABLE_STRING_8; a_body: detachable READABLE_STRING_8) diff --git a/library/network/http_client/src/http_client_session.e b/library/network/http_client/src/http_client_session.e index e2f8267d..832dc4c5 100644 --- a/library/network/http_client/src/http_client_session.e +++ b/library/network/http_client/src/http_client_session.e @@ -50,6 +50,14 @@ feature {NONE} -- Initialization feature -- Basic operation + close + -- Close session. + --| useful to disconnect persistent connection. + do + end + +feature -- Access + url (a_path: READABLE_STRING_8; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT): STRING_8 -- Url computed from Current and `ctx' data. local @@ -88,17 +96,23 @@ feature -- Helper get (a_path: READABLE_STRING_8; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT): HTTP_CLIENT_RESPONSE -- Response for GET request based on Current, `a_path' and `ctx'. + require + is_available: is_available deferred end head (a_path: READABLE_STRING_8; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT): HTTP_CLIENT_RESPONSE -- Response for HEAD request based on Current, `a_path' and `ctx'. + require + is_available: is_available deferred end post (a_path: READABLE_STRING_8; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT; data: detachable READABLE_STRING_8): HTTP_CLIENT_RESPONSE -- Response for POST request based on Current, `a_path' and `ctx' -- with input `data' + require + is_available: is_available deferred end @@ -111,29 +125,39 @@ feature -- Helper patch (a_path: READABLE_STRING_8; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT; data: detachable READABLE_STRING_8): HTTP_CLIENT_RESPONSE -- Response for PATCH request based on Current, `a_path' and `ctx' -- with input `data' + require + is_available: is_available deferred end patch_file (a_path: READABLE_STRING_8; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT; fn: detachable READABLE_STRING_8): HTTP_CLIENT_RESPONSE -- Response for PATCH request based on Current, `a_path' and `ctx' -- with uploaded data file `fn' + require + is_available: is_available deferred end put (a_path: READABLE_STRING_8; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT; data: detachable READABLE_STRING_8): HTTP_CLIENT_RESPONSE -- Response for PUT request based on Current, `a_path' and `ctx' -- with input `data' + require + is_available: is_available deferred end put_file (a_path: READABLE_STRING_8; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT; fn: detachable READABLE_STRING_8): HTTP_CLIENT_RESPONSE -- Response for PUT request based on Current, `a_path' and `ctx' -- with uploaded file `fn' + require + is_available: is_available deferred end delete (a_path: READABLE_STRING_8; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT): HTTP_CLIENT_RESPONSE -- Response for DELETE request based on Current, `a_path' and `ctx' + require + is_available: is_available deferred end diff --git a/library/network/http_client/src/spec/net/implementation/net_http_client_connection.e b/library/network/http_client/src/spec/net/implementation/net_http_client_connection.e new file mode 100644 index 00000000..df2ace03 --- /dev/null +++ b/library/network/http_client/src/spec/net/implementation/net_http_client_connection.e @@ -0,0 +1,52 @@ +note + description: "Socket connection information, used for peristent connection implementation." + date: "$Date$" + revision: "$Revision$" + +class + NET_HTTP_CLIENT_CONNECTION + +create + make + +feature {NONE} -- Initialization + + make (a_socket: NETWORK_STREAM_SOCKET; a_host: READABLE_STRING_GENERAL; a_port: INTEGER) + do + socket := a_socket + host := a_host + port := a_port + end + +feature -- Access + + socket: NETWORK_STREAM_SOCKET + -- Persistent connection socket. + + host: READABLE_STRING_GENERAL + -- Host used for this connection. + + port: INTEGER + -- Port used for this connection. + +feature -- Status report + + is_reusable (a_host: READABLE_STRING_GENERAL; a_port: INTEGER): BOOLEAN + -- Is Current connection reusable for new connection `a_host:a_port'? + do + if a_host.same_string (host) and port = a_port then + Result := socket.is_connected + end + end + +note + copyright: "2011-2015, Jocelyn Fiat, Javier Velilla, Eiffel Software and others" + license: "Eiffel Forum License v2 (see http://www.eiffel.com/licensing/forum.txt)" + source: "[ + Eiffel Software + 5949 Hollister Ave., Goleta, CA 93117 USA + Telephone 805-685-1006, Fax 805-685-6869 + Website http://www.eiffel.com + Customer support http://support.eiffel.com + ]" +end diff --git a/library/network/http_client/src/spec/net/net_http_client_request.e b/library/network/http_client/src/spec/net/net_http_client_request.e index 5e8d1ac2..012c2560 100644 --- a/library/network/http_client/src/spec/net/net_http_client_request.e +++ b/library/network/http_client/src/spec/net/net_http_client_request.e @@ -28,13 +28,44 @@ feature {NONE} -- Internal session: NET_HTTP_CLIENT_SESSION net_http_client_version: STRING = "0.1" - - new_socket (a_host: READABLE_STRING_8; a_port: INTEGER; a_is_https: BOOLEAN; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT): NETWORK_STREAM_SOCKET + session_socket (a_host: READABLE_STRING_8; a_port: INTEGER; a_is_https: BOOLEAN; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT): NETWORK_STREAM_SOCKET + -- Session socket to use for connection. + -- Eventually reuse the persistent connection if any. + local + l_socket: detachable NETWORK_STREAM_SOCKET do - if a_is_https then - create {SSL_NETWORK_STREAM_SOCKET} Result.make_client_by_port (a_port, a_host) + if + attached session.persistent_connection as l_persistent_connection and then + l_persistent_connection.is_reusable (a_host, a_port) + then + l_socket := l_persistent_connection.socket + if a_is_https then + if attached {SSL_NETWORK_STREAM_SOCKET} l_socket as l_ssl_socket then + Result := l_ssl_socket + else + l_socket := Void + end + elseif attached {SSL_NETWORK_STREAM_SOCKET} l_socket as l_ssl_socket then + l_socket := Void + end + if l_socket /= Void and then not l_socket.is_connected then + -- Reset persistent connection + l_socket := Void + end + end + if l_socket /= Void then + -- Reuse persistent connection. + Result := l_socket else - create Result.make_client_by_port (a_port, a_host) + session.set_persistent_connection (Void) + if a_is_https then + create {SSL_NETWORK_STREAM_SOCKET} Result.make_client_by_port (a_port, a_host) + else + create Result.make_client_by_port (a_port, a_host) + end + Result.set_connect_timeout (connect_timeout) + Result.set_timeout (timeout) + Result.connect end end @@ -67,13 +98,14 @@ feature -- Access l_form_string: STRING l_prev_header: READABLE_STRING_8 l_boundary: READABLE_STRING_8 - l_is_http_1_0: BOOLEAN + l_is_http_1_0_request: BOOLEAN + l_is_keep_alive: BOOLEAN retried: BOOLEAN do if not retried then ctx := context if ctx /= Void then - l_is_http_1_0 := attached ctx.http_version as l_http_version and then l_http_version.same_string ("HTTP/1.0") + l_is_http_1_0_request := attached ctx.http_version as l_http_version and then l_http_version.same_string ("HTTP/1.0") end create Result.make (url) @@ -96,10 +128,7 @@ feature -- Access end -- Connect - l_socket := new_socket (l_host, l_port, l_is_https, ctx) - l_socket.set_connect_timeout (connect_timeout) - l_socket.set_timeout (timeout) - l_socket.connect + l_socket := session_socket (l_host, l_port, l_is_https, ctx) if l_socket.is_connected then create l_form_string.make_empty @@ -197,7 +226,7 @@ feature -- Access s.append_character (' ') s.append (l_request_uri) s.append_character (' ') - if l_is_http_1_0 then + if l_is_http_1_0_request then s.append ("HTTP/1.0") else s.append ("HTTP/1.1") @@ -213,12 +242,12 @@ feature -- Access s.append (l_host) end s.append (http_end_of_header_line) +-- s.append ("Connection: close") +-- s.append (http_end_of_header_line) -- Append the given request headers l_cookie := Void - if headers.is_empty then - s.append (Http_end_of_command) - else + if not headers.is_empty then across headers as ic loop @@ -235,11 +264,10 @@ feature -- Access s.append (Http_end_of_header_line) end end - s.append (Http_end_of_header_line) end -- Compute Header Cookie: if needed - -- Get session cookie + -- Use session cookie if l_cookie = Void then l_cookie := session.cookie else @@ -255,6 +283,10 @@ feature -- Access s.append (l_upload_data) s.append (http_end_of_header_line) end + + --| End of client header. + s.append (Http_end_of_header_line) + --| Note that any remaining file to upload will be done directly via the socket --| to optimize memory usage @@ -270,6 +302,9 @@ feature -- Access --| Send request |-- --|-------------------------|-- + debug ("socket_header") + io.error.put_string (s) + end l_socket.put_string (s) --| Send remaining payload data, if needed. if l_upload_file /= Void then @@ -283,35 +318,57 @@ feature -- Access --|-------------------------|-- if l_socket.ready_for_reading then create l_message.make_empty - append_socket_header_content_to (l_socket, l_message) + append_socket_header_content_to (Result, l_socket, l_message) l_prev_header := Result.raw_header Result.set_raw_header (l_message.string) - l_content_length := -1 - if attached Result.header ("Content-Length") as s_len and then s_len.is_integer then - l_content_length := s_len.to_integer - end - l_location := Result.header ("Location") - if attached Result.header ("Set-Cookie") as s_cookies then - session.set_cookie (s_cookies) - end l_message.append (http_end_of_header_line) - -- Get content if any. - append_socket_content_to (Result, l_socket, l_content_length, l_message) - -- Restore previous header - Result.set_raw_header (l_prev_header) - -- Set message - Result.set_response_message (l_message, ctx) - -- Check status code. - check status_coherent: attached Result.status_line as l_status_line implies l_status_line.has_substring (Result.status.out) end + if not Result.error_occurred then + -- Get information from header + l_content_length := -1 + if attached Result.header ("Content-Length") as s_len and then s_len.is_integer then + l_content_length := s_len.to_integer + end + l_location := Result.header ("Location") + if attached Result.header ("Set-Cookie") as s_cookies then + session.set_cookie (s_cookies) + end - -- follow redirect - if l_location /= Void then - if Result.redirections_count < max_redirects then - initialize (l_location, ctx) - redirection_response := response - redirection_response.add_redirection (Result.status_line, Result.raw_header, Result.body) - Result := redirection_response + -- Keep-alive connection? + -- with HTTP/1.1, this is the default, and could be changed by Connection: close + -- with HTTP/1.0, it requires "Connection: keep-alive" header line. + if attached Result.header ("Connection") as s_connection then + l_is_keep_alive := s_connection.same_string ("keep-alive") + else + l_is_keep_alive := not Result.is_http_1_0 + end + + -- Get content if any. + append_socket_content_to (Result, l_socket, l_content_length, l_message) + -- Restore previous header + Result.set_raw_header (l_prev_header) + -- Set message + Result.set_response_message (l_message, ctx) + -- Check status code. + check status_coherent: attached Result.status_line as l_status_line implies l_status_line.has_substring (Result.status.out) end + + if l_is_keep_alive then + session.set_persistent_connection (create {NET_HTTP_CLIENT_CONNECTION}.make (l_socket, l_host, l_port)) + else + session.set_persistent_connection (Void) + end + + -- follow redirect + if l_location /= Void then + if Result.redirections_count < max_redirects then + initialize (l_location, ctx) + redirection_response := response + redirection_response.add_redirection (Result.status_line, Result.raw_header, Result.body) + Result := redirection_response + end + end + if not l_is_keep_alive then + l_socket.cleanup end end else @@ -477,7 +534,7 @@ feature {NONE} -- Helpers end end - append_socket_header_content_to (a_socket: NETWORK_STREAM_SOCKET; a_output: STRING) + append_socket_header_content_to (a_response: HTTP_CLIENT_RESPONSE; a_socket: NETWORK_STREAM_SOCKET; a_output: STRING) -- Get header from `a_socket' into `a_output'. local s: READABLE_STRING_8 @@ -485,16 +542,30 @@ feature {NONE} -- Helpers from s := "" until - s.same_string ("%R") or not a_socket.readable + s.same_string ("%R") or not a_socket.readable or a_response.error_occurred loop - a_socket.read_line_thread_aware - s := a_socket.last_string - if s.same_string ("%R") then - -- Reach end of header --- a_output.append (http_end_of_header_line) + if a_socket.ready_for_reading then + a_socket.read_line_thread_aware + s := a_socket.last_string + debug ("socket_header") + io.error.put_string ("header-line: " + s + "%N") + end + if s.is_empty then + debug ("socket_header") + io.error.put_string ("ERROR: zero byte read when receiving header.%N") + end + a_response.set_error_message ("Read zero byte, expecting header line") + elseif s.same_string ("%R") then + -- Reach end of header + else + a_output.append (s) + a_output.append_character ('%N') + end else - a_output.append (s) - a_output.append_character ('%N') + debug ("socket_header") + io.error.put_string ("ERROR: timeout when receiving header.%N") + end + a_response.set_error_message ("Could not read header data, timeout") end end end diff --git a/library/network/http_client/src/spec/net/net_http_client_session.e b/library/network/http_client/src/spec/net/net_http_client_session.e index 6a6653f5..7fab395f 100644 --- a/library/network/http_client/src/spec/net/net_http_client_session.e +++ b/library/network/http_client/src/spec/net/net_http_client_session.e @@ -10,6 +10,9 @@ class inherit HTTP_CLIENT_SESSION + redefine + close + end NET_HTTP_CLIENT_INFO @@ -33,6 +36,30 @@ feature -- Status report end end +feature -- Access + + persistent_connection: detachable NET_HTTP_CLIENT_CONNECTION + -- Socket used for persistent connection purpose. + +feature -- Element change + + set_persistent_connection (a_connection: like persistent_connection) + -- Set `persistent_connection' to `a_connection'. + do + persistent_connection := a_connection + end + +feature -- Basic operation + + close + -- + do + if attached persistent_connection as l_connection then + persistent_connection := Void + l_connection.socket.cleanup + end + end + feature -- Custom custom (a_method: READABLE_STRING_8; a_path: READABLE_STRING_8; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT): HTTP_CLIENT_RESPONSE diff --git a/library/network/http_client/tests/test_net_http_client.e b/library/network/http_client/tests/test_net_http_client.e index d906d334..c96f83ca 100644 --- a/library/network/http_client/tests/test_net_http_client.e +++ b/library/network/http_client/tests/test_net_http_client.e @@ -37,6 +37,50 @@ feature -- Tests test_headers end + test_persistent_connection + local + sess: like new_session + h: STRING_8 + do + sess := new_session ("http://www.google.fr") + if attached sess.get ("/", Void) as res then + assert ("Get returned without error", not res.error_occurred) + create h.make_empty + if attached res.headers as hds then + across + hds as c + loop + h.append (c.item.name + ": " + c.item.value + "%R%N") + end + end + if attached res.body as l_body then + assert ("body not empty", not l_body.is_empty) + else + assert ("missing body", False) + end + assert ("same headers", h.same_string (res.raw_header)) + end + if attached sess.get ("/", Void) as res then + assert ("Get returned without error", not res.error_occurred) + create h.make_empty + if attached res.headers as hds then + across + hds as c + loop + h.append (c.item.name + ": " + c.item.value + "%R%N") + end + end + if attached res.body as l_body then + assert ("body not empty", not l_body.is_empty) + else + assert ("missing body", False) + end + assert ("same headers", h.same_string (res.raw_header)) + end + sess.close + end + + end