From b64a281d75e4aa98b4a51b25a35fb1ca3168f596 Mon Sep 17 00:00:00 2001 From: Jocelyn Fiat Date: Thu, 17 Sep 2015 22:48:04 +0200 Subject: [PATCH] Fixed timeout issue due to too many "ready_for_reading". Fixed Connection behavior. Fixed Content-Type settings. Removed condition on POST or PUT, since code also applied to any request methods. Added verbose output implementation. --- .../network/http_client/http_client-safe.ecf | 6 +- library/network/http_client/http_client.ecf | 6 +- .../http_client/src/http_client_session.e | 55 +++++ .../libcurl/libcurl_http_client_request.e | 37 ++++ .../src/spec/net/net_http_client_request.e | 204 +++++++++--------- .../src/spec/net/net_http_client_session.e | 4 +- 6 files changed, 198 insertions(+), 114 deletions(-) diff --git a/library/network/http_client/http_client-safe.ecf b/library/network/http_client/http_client-safe.ecf index fc4bd799..7eb88979 100644 --- a/library/network/http_client/http_client-safe.ecf +++ b/library/network/http_client/http_client-safe.ecf @@ -18,11 +18,7 @@ - - - - - + diff --git a/library/network/http_client/http_client.ecf b/library/network/http_client/http_client.ecf index 984b686e..a5e28478 100644 --- a/library/network/http_client/http_client.ecf +++ b/library/network/http_client/http_client.ecf @@ -18,11 +18,7 @@ - - - - - + diff --git a/library/network/http_client/src/http_client_session.e b/library/network/http_client/src/http_client_session.e index 832dc4c5..b5bf34c5 100644 --- a/library/network/http_client/src/http_client_session.e +++ b/library/network/http_client/src/http_client_session.e @@ -85,6 +85,61 @@ feature -- Access end end +feature {NONE} -- Access: verbose + + verbose_mode: INTEGER + -- Internal verbose mode. + + verbose_header_sent_mode: INTEGER = 1 --| 0001 + verbose_header_received_mode: INTEGER = 2 --| 0010 + verbose_debug_mode: INTEGER = 4 --| 0100 + +feature -- Access: verbose + + is_header_sent_verbose: BOOLEAN + do + Result := verbose_mode & verbose_header_sent_mode = verbose_header_sent_mode + end + + is_header_received_verbose: BOOLEAN + do + Result := verbose_mode & verbose_header_received_mode = verbose_header_received_mode + end + + is_debug_verbose: BOOLEAN + do + Result := verbose_mode & verbose_debug_mode = verbose_debug_mode + end + +feature -- Element change: verbose + + set_header_sent_verbose (b: BOOLEAN) + do + if b then + verbose_mode := verbose_mode | verbose_header_sent_mode + else + verbose_mode := verbose_mode & verbose_header_sent_mode.bit_not + end + end + + set_header_received_verbose (b: BOOLEAN) + do + if b then + verbose_mode := verbose_mode | verbose_header_received_mode + else + verbose_mode := verbose_mode & verbose_header_received_mode.bit_not + end + end + + set_debug_verbose (b: BOOLEAN) + do + if b then + verbose_mode := verbose_mode | verbose_debug_mode + else + verbose_mode := verbose_mode & verbose_debug_mode.bit_not + 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/src/spec/libcurl/libcurl_http_client_request.e b/library/network/http_client/src/spec/libcurl/libcurl_http_client_request.e index c927f495..3f605ea0 100644 --- a/library/network/http_client/src/spec/libcurl/libcurl_http_client_request.e +++ b/library/network/http_client/src/spec/libcurl/libcurl_http_client_request.e @@ -62,6 +62,7 @@ feature -- Execution l_upload_filename: detachable READABLE_STRING_GENERAL l_headers: like headers l_is_http_1_0: BOOLEAN + l_uri: URI do if not retried then curl := session.curl @@ -84,6 +85,38 @@ feature -- Execution append_parameters_to_url (ctx.query_parameters, l_url) end + if session.is_header_sent_verbose then + io.error.put_string ("> Sending:%N") + create l_uri.make_from_string (l_url) + io.error.put_string ("> ") + io.error.put_string (request_method + " " + l_uri.path) + if attached l_uri.query as q then + io.error.put_string (q) + end + if l_is_http_1_0 then + io.error.put_string (" HTTP/1.0") + else + io.error.put_string (" HTTP/1.1") + end + io.error.put_new_line + if attached l_uri.host as l_host then + io.error.put_string ("> ") + io.error.put_string ("Host: " + l_host) + io.error.put_new_line + end + across + headers as ic + loop + io.error.put_string ("> ") + io.error.put_string (ic.key) + io.error.put_string (": ") + io.error.put_string (ic.item) + io.error.put_new_line + end + io.error.put_string ("> ... ") + io.error.put_new_line + end + debug ("service") io.put_string ("SERVICE: " + l_url) io.put_new_line @@ -248,6 +281,10 @@ feature -- Execution Result.status := response_status_code (curl_easy, curl_handle) if l_curl_string /= Void then Result.set_response_message (l_curl_string.string, ctx) + if session.is_header_received_verbose then + io.error.put_string ("< Receiving:%N") + io.error.put_string (Result.raw_header) + end end else Result.set_error_message ("Error: cURL Error[" + l_result.out + "]") 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 012c2560..61cc4ae7 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 @@ -127,7 +127,14 @@ feature -- Access l_host := l_url.host end - -- Connect + if attached session.proxy as l_proxy_settings then + -- For now, so proxy support. + check + not_supported: False + end + end + + -- Connect l_socket := session_socket (l_host, l_port, l_is_https, ctx) if l_socket.is_connected then @@ -194,27 +201,23 @@ feature -- Access l_boundary := new_mime_boundary headers.extend ("multipart/form-data; boundary=" + l_boundary, "Content-Type") if l_form_data /= Void then - headers.extend ("*/*", "Accept") l_upload_data := form_date_and_uploaded_files_to_mime_string (l_form_data, l_upload_filename, l_boundary) headers.extend (l_upload_data.count.out, "Content-Length") end end - elseif - request_method.is_case_insensitive_equal ("POST") - or else request_method.is_case_insensitive_equal ("PUT") - then - if l_upload_data /= Void then - check ctx.has_upload_data end + elseif l_upload_data /= Void then + check ctx.has_upload_data end + if not headers.has ("Content-Type") then headers.extend ("application/x-www-form-urlencoded", "Content-Type") - headers.extend (l_upload_data.count.out, "Content-Length") - elseif l_upload_filename /= Void then - check ctx.has_upload_filename end - create l_upload_file.make_with_name (l_upload_filename) - if l_upload_file.exists and then l_upload_file.readable then - headers.extend (l_upload_file.count.out, "Content-Length") - end - check l_upload_file /= Void end end + headers.extend (l_upload_data.count.out, "Content-Length") + elseif l_upload_filename /= Void then + check ctx.has_upload_filename end + create l_upload_file.make_with_name (l_upload_filename) + if l_upload_file.exists and then l_upload_file.readable then + headers.extend (l_upload_file.count.out, "Content-Length") + end + check l_upload_file /= Void end end end @@ -242,8 +245,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) + if not headers.has ("Connection") then + if l_is_http_1_0_request then + s.append ("Connection: keep-alive") + s.append (http_end_of_header_line) + end + end -- Append the given request headers l_cookie := Void @@ -279,14 +286,14 @@ feature -- Access s.append (http_end_of_header_line) end + --| End of client header. + s.append (Http_end_of_header_line) + if l_upload_data /= Void then 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 @@ -302,8 +309,9 @@ feature -- Access --| Send request |-- --|-------------------------|-- - debug ("socket_header") - io.error.put_string (s) + if session.is_header_sent_verbose then + log ("> Sending:%N") + log (s) end l_socket.put_string (s) --| Send remaining payload data, if needed. @@ -316,9 +324,13 @@ feature -- Access --| Get response. |-- --| Get header message |-- --|-------------------------|-- - if l_socket.ready_for_reading then + if is_ready_for_reading (l_socket) then create l_message.make_empty append_socket_header_content_to (Result, l_socket, l_message) + if session.is_header_received_verbose then + log ("< Receiving:%N") + log (l_message) + end l_prev_header := Result.raw_header Result.set_raw_header (l_message.string) l_message.append (http_end_of_header_line) @@ -372,12 +384,21 @@ feature -- Access end end else + if session.is_debug_verbose then + log ("Debug: Read Timeout!%N") + end Result.set_error_message ("Read Timeout") end else + if session.is_debug_verbose then + log ("Debug: Write Timeout!%N") + end Result.set_error_message ("Write Timeout") end else + if session.is_debug_verbose then + log ("Debug: Could not connect!%N") + end Result.set_error_message ("Could not connect") end else @@ -391,7 +412,21 @@ feature -- Access feature {NONE} -- Helpers + log (m: READABLE_STRING_8) + -- Output log messages. + do + io.error.put_string (m) + end + + is_ready_for_reading (a_socket: NETWORK_STREAM_SOCKET): BOOLEAN + -- Is `a_socket' ready for reading? + do + Result := a_socket.ready_for_reading + end + form_date_and_uploaded_files_to_mime_string (a_form_parameters: HASH_TABLE [READABLE_STRING_32, READABLE_STRING_32]; a_upload_filename: detachable READABLE_STRING_GENERAL; a_mime_boundary: READABLE_STRING_8): STRING + -- Form data and uploaded files converted to mime string. + -- TODO: design a proper MIME... component. local l_path: PATH l_mime_type: READABLE_STRING_8 @@ -544,28 +579,18 @@ feature {NONE} -- Helpers until s.same_string ("%R") or not a_socket.readable or a_response.error_occurred loop - 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') + a_socket.read_line_thread_aware + s := a_socket.last_string + if s.is_empty then + if session.is_debug_verbose then + log ("Debug: 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 - 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") + a_output.append (s) + a_output.append_character ('%N') end end end @@ -581,29 +606,22 @@ feature {NONE} -- Helpers do if a_socket.readable then if a_len >= 0 then - debug ("socket_content") - io.error.put_string ("Content-Length="+ a_len.out +"%N") + if session.is_debug_verbose then + log ("Debug: Content-Length="+ a_len.out +"%N") end from r := a_len until r = 0 or else not a_socket.readable or else a_response.error_occurred loop - if a_socket.ready_for_reading then - a_socket.read_stream_thread_aware (r) - l_count := l_count + a_socket.bytes_read - debug ("socket_content") - io.error.put_string (" - byte read=" + a_socket.bytes_read.out + "%N") - io.error.put_string (" - current count=" + l_count.out + "%N") - end - r := r - a_socket.bytes_read - a_output.append (a_socket.last_string) - else - debug ("socket_content") - io.error.put_string (" -! TIMEOUT%N") - end - a_response.set_error_message ("Could not read chunked data, timeout") + a_socket.read_stream_thread_aware (r) + l_count := l_count + a_socket.bytes_read + if session.is_debug_verbose then + log ("Debug: - byte read=" + a_socket.bytes_read.out + "%N") + log ("Debug: - current count=" + l_count.out + "%N") end + r := r - a_socket.bytes_read + a_output.append (a_socket.last_string) end check full_content_read: not a_response.error_occurred implies l_count = a_len end elseif attached a_response.header ("Transfer-Encoding") as l_enc and then l_enc.is_case_insensitive_equal ("chunked") then @@ -619,15 +637,11 @@ feature {NONE} -- Helpers until n < l_chunk_size or not a_socket.readable loop - if a_socket.ready_for_reading then - a_socket.read_stream_thread_aware (l_chunk_size) - s := a_socket.last_string - n := a_socket.bytes_read - l_count := l_count + n - a_output.append (s) - else - a_response.set_error_message ("Could not read data, timeout") - end + a_socket.read_stream_thread_aware (l_chunk_size) + s := a_socket.last_string + n := a_socket.bytes_read + l_count := l_count + n + a_output.append (s) end end end @@ -645,8 +659,8 @@ feature {NONE} -- Helpers n,pos, l_count: INTEGER hexa2int: HEXADECIMAL_STRING_TO_INTEGER_CONVERTER do - debug ("socket_content") - io.error.put_string ("Chunked encoding%N") + if session.is_debug_verbose then + log ("Debug: Chunked encoding%N") end from create hexa2int.make @@ -657,8 +671,8 @@ feature {NONE} -- Helpers a_socket.read_line_thread_aware -- Read chunk info s := a_socket.last_string s.right_adjust - debug ("socket_content") - io.error.put_string (" - chunk info='" + s + "'%N") + if session.is_debug_verbose then + log ("Debug: - chunk info='" + s + "'%N") end pos := s.index_of (';', 1) if pos > 0 then @@ -674,8 +688,8 @@ feature {NONE} -- Helpers n := 0 end end - debug ("socket_content") - io.error.put_string (" - chunk size=" + n.out + "%N") + if session.is_debug_verbose then + log ("Debug: - chunk size=" + n.out + "%N") end if n > 0 then from @@ -683,36 +697,22 @@ feature {NONE} -- Helpers until r = 0 or else not a_socket.readable or else a_response.error_occurred loop - if a_socket.ready_for_reading then - a_socket.read_stream_thread_aware (r) - l_count := l_count + a_socket.bytes_read - debug ("socket_content") - io.error.put_string (" - byte read=" + a_socket.bytes_read.out + "%N") - io.error.put_string (" - current count=" + l_count.out + "%N") - end - r := r - a_socket.bytes_read - a_output.append (a_socket.last_string) - else - debug ("socket_content") - io.error.put_string (" -! TIMEOUT%N") - end - a_response.set_error_message ("Could not read chunked data, timeout") + a_socket.read_stream_thread_aware (r) + l_count := l_count + a_socket.bytes_read + if session.is_debug_verbose then + log ("Debug: - byte read=" + a_socket.bytes_read.out + "%N") + log ("Debug: - current count=" + l_count.out + "%N") end + r := r - a_socket.bytes_read + a_output.append (a_socket.last_string) end - if a_socket.ready_for_reading then - a_socket.read_character - check a_socket.last_character = '%R' end - a_socket.read_character - check a_socket.last_character = '%N' end - debug ("socket_content") - io.error.put_string (" - Found CRNL %N") - end - else - debug ("socket_content") - io.error.put_string (" -! TIMEOUT%N") - end - a_response.set_error_message ("Could not read chunked data, timeout") + a_socket.read_character + check a_socket.last_character = '%R' end + a_socket.read_character + check a_socket.last_character = '%N' end + if session.is_debug_verbose then + log ("Debug: - Found CRNL %N") 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 7fab395f..a5a6349a 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 @@ -36,12 +36,12 @@ feature -- Status report end end -feature -- Access +feature -- Access: persistent connection persistent_connection: detachable NET_HTTP_CLIENT_CONNECTION -- Socket used for persistent connection purpose. -feature -- Element change +feature -- Element change: persistent connection set_persistent_connection (a_connection: like persistent_connection) -- Set `persistent_connection' to `a_connection'.