[qpid-dispatch] branch master updated (ceb19cf -> a14b289)

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[qpid-dispatch] branch master updated (ceb19cf -> a14b289)

kgiusti
This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git.


    from ceb19cf  DISPATCH-1904: Removed unused function qd_entity_set_stringf which was causing compilation error. This closes #969.
     new d0caeb6  DISPATCH-1744: bugfix - do not finish response if status is 1xx
     new bae8bcf  DISPATCH-1744: bugfix - properly set outcome on invalid response messages
     new d196647  DISPATCH-1880: avoid discarding invalid messages on outbound link
     new 4934705  DISPATCH-1744: bugfix - do not call request_complete if close expected
     new 45198bf  NO-JIRA: clarify delivery inc/decref messages
     new 7515af4  DISPATCH-1744: clear close expected flag when close occurs
     new 125b7d5  DISPATCH-1744: discard outgoing request on raw conn close
     new 8061881  DISPATCH-1744: re-factor how requests are retired
     new a14b289  DISPATCH-1744: remove 100 Continue test on HTTP/1.0 server

The 9 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 include/qpid/dispatch/http1_codec.h |   8 +-
 src/adaptors/http1/http1_client.c   |  47 +++---
 src/adaptors/http1/http1_codec.c    |  27 +--
 src/adaptors/http1/http1_server.c   | 319 +++++++++++++++++++-----------------
 src/router_node.c                   |   2 +-
 tests/system_tests_http1_adaptor.py |   8 +-
 6 files changed, 221 insertions(+), 190 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[qpid-dispatch] 01/09: DISPATCH-1744: bugfix - do not finish response if status is 1xx

kgiusti
This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git

commit d0caeb6e0fdfa3091992b854227c7d2ce8fafb11
Author: Kenneth Giusti <[hidden email]>
AuthorDate: Thu Dec 10 16:03:07 2020 -0500

    DISPATCH-1744: bugfix - do not finish response if status is 1xx
---
 src/adaptors/http1/http1_codec.c  | 7 +++++--
 src/adaptors/http1/http1_server.c | 7 +++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/adaptors/http1/http1_codec.c b/src/adaptors/http1/http1_codec.c
index 6c03e7f..ffa8a9f 100644
--- a/src/adaptors/http1/http1_codec.c
+++ b/src/adaptors/http1/http1_codec.c
@@ -1344,10 +1344,13 @@ void h1_codec_connection_rx_closed(h1_codec_connection_t *conn)
         struct decoder_t *decoder = &conn->decoder;
         h1_codec_request_state_t *hrs = decoder->hrs;
         if (hrs) {
-            // consider the response valid if length is unspecified since in
+            // consider the response complete if length is unspecified since in
             // this case the server must close the connection to complete the
-            // message body
+            // message body.  However if the response message is a "continue"
+            // then the final response never arrived and the response is
+            // incomplete
             if (decoder->state == HTTP1_MSG_STATE_BODY
+                && !IS_INFO_RESPONSE(hrs->response_code)
                 && !decoder->is_chunked
                 && !decoder->hdr_content_length) {
 
diff --git a/src/adaptors/http1/http1_server.c b/src/adaptors/http1/http1_server.c
index 706c1e3..cffcb5c 100644
--- a/src/adaptors/http1/http1_server.c
+++ b/src/adaptors/http1/http1_server.c
@@ -636,7 +636,7 @@ static bool _process_requests(qdr_http1_connection_t *hconn)
             if ((!hreq->request_acked || !hreq->request_settled) &&
                 hconn->cfg.aggregation == QD_AGGREGATION_NONE) {
 
-                if (hreq->request_dispo == 0)
+                if (!hreq->request_dispo || hreq->request_dispo == PN_ACCEPTED)
                     hreq->request_dispo = (hreq->base.out_http1_octets > 0
                                            ? PN_MODIFIED : PN_RELEASED);
 
@@ -990,7 +990,6 @@ static void _server_rx_done_cb(h1_codec_request_state_t *hrs)
            "[C%"PRIu64"][L%"PRIu64"] HTTP response message msg-id=%"PRIu64" decoding complete.",
            hconn->conn_id, hconn->in_link_id, hreq->base.msg_id);
 
-    hreq->response_complete = true;
     rmsg->rx_complete = true;
 
     if (!qd_message_receive_complete(msg)) {
@@ -1004,6 +1003,10 @@ static void _server_rx_done_cb(h1_codec_request_state_t *hrs)
         // We've finished the delivery, and don't care about outcome/settlement
         _server_response_msg_free(hreq, rmsg);
     }
+
+    // only consider the response complete if terminal response code (!1xx)
+    if (h1_codec_request_state_response_code(hrs) / 100 != 1)
+        hreq->response_complete = true;
 }
 
 


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[qpid-dispatch] 02/09: DISPATCH-1744: bugfix - properly set outcome on invalid response messages

kgiusti
In reply to this post by kgiusti
This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git

commit bae8bcf615fa3ce8fa5e8061f5de377f211501f7
Author: Kenneth Giusti <[hidden email]>
AuthorDate: Thu Dec 10 17:20:24 2020 -0500

    DISPATCH-1744: bugfix - properly set outcome on invalid response messages
---
 src/adaptors/http1/http1_client.c | 36 +++++++++++++++++++++++++-----------
 src/adaptors/http1/http1_server.c |  5 ++++-
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/src/adaptors/http1/http1_client.c b/src/adaptors/http1/http1_client.c
index 4093c10..952c992 100644
--- a/src/adaptors/http1/http1_client.c
+++ b/src/adaptors/http1/http1_client.c
@@ -487,16 +487,18 @@ static void _handle_connection_events(pn_event_t *e, qd_server_t *qd_server, voi
                    DEQ_IS_EMPTY(rmsg->out_data.fifo) &&
                    hconn->cfg.aggregation == QD_AGGREGATION_NONE) {
                 // response message fully received and forwarded to client
-                qd_log(qdr_http1_adaptor->log, QD_LOG_TRACE,
-                       "[C%"PRIu64"][L%"PRIu64"] HTTP client request msg-id=%"PRIu64" settling response, dispo=0x%"PRIx64,
-                       hconn->conn_id, hconn->out_link_id, hreq->base.msg_id, rmsg->dispo);
-                qdr_delivery_remote_state_updated(qdr_http1_adaptor->core,
-                                                  rmsg->dlv,
-                                                  rmsg->dispo,
-                                                  true,   // settled,
-                                                  0,      // error
-                                                  0,      // dispo data
-                                                  false);
+                if (rmsg->dlv) {
+                    qd_log(qdr_http1_adaptor->log, QD_LOG_TRACE,
+                           "[C%"PRIu64"][L%"PRIu64"] HTTP client request msg-id=%"PRIu64" settling response, dispo=0x%"PRIx64,
+                           hconn->conn_id, hconn->out_link_id, hreq->base.msg_id, rmsg->dispo);
+                    qdr_delivery_remote_state_updated(qdr_http1_adaptor->core,
+                                                      rmsg->dlv,
+                                                      rmsg->dispo,
+                                                      true,   // settled,
+                                                      0,      // error
+                                                      0,      // dispo data
+                                                      false);
+                }
                 qdr_link_flow(qdr_http1_adaptor->core, hconn->out_link, 1, false);
                 _client_response_msg_free(hreq, rmsg);
                 rmsg = DEQ_HEAD(hreq->responses);
@@ -1208,7 +1210,7 @@ void qdr_http1_client_core_delivery_update(qdr_http1_adaptor_t      *adaptor,
                    "[C%"PRIu64"][L%"PRIu64"] HTTP request msg-id=%"PRIu64" failure, outcome=0x%"PRIx64,
                    hconn->conn_id, hconn->in_link_id, hreq->base.msg_id, disp);
 
-            if (DEQ_IS_EMPTY(hreq->responses)) {
+            if (hreq->base.out_http1_octets == 0) {
                 // best effort attempt to send an error to the client
                 // if nothing has been sent back so far
                 _client_response_msg_t *rmsg = new__client_response_msg_t();
@@ -1535,11 +1537,23 @@ uint64_t qdr_http1_client_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
                 bool need_close = false;
                 h1_codec_tx_done(hreq->base.lib_rs, &need_close);
                 hreq->close_on_complete = need_close || hreq->close_on_complete;
+
+                qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG,
+                       "[C%"PRIu64"][L%"PRIu64"] HTTP response message msg-id=%"PRIu64" encoding complete",
+                       hconn->conn_id, link->identity, hreq->base.msg_id);
+
             } else {
                 // The response was bad.  There's not much that can be done to
                 // recover, so for now I punt...
                 qd_message_set_discard(msg, true);
+
+                // returning a terminal disposition will cause the delivery to be updated and settled,
+                // so drop our reference
+                qdr_delivery_set_context(rmsg->dlv, 0);
+                qdr_delivery_decref(qdr_http1_adaptor->core, rmsg->dlv, "malformed HTTP1 response, delivery released");
+                rmsg->dlv = 0;
                 qdr_http1_close_connection(hconn, "Cannot parse response message");
+                return rmsg->dispo;
             }
         }
     }
diff --git a/src/adaptors/http1/http1_server.c b/src/adaptors/http1/http1_server.c
index cffcb5c..8f03fe7 100644
--- a/src/adaptors/http1/http1_server.c
+++ b/src/adaptors/http1/http1_server.c
@@ -1449,7 +1449,10 @@ uint64_t qdr_http1_server_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
         if (hreq->request_dispo) {
             qd_message_set_send_complete(msg);
             if (hreq->request_dispo == PN_ACCEPTED) {
-                h1_codec_tx_done(hreq->base.lib_rs, &hreq->close_on_complete);
+                bool need_close = false;
+                h1_codec_tx_done(hreq->base.lib_rs, &need_close);
+                hreq->close_on_complete = need_close || hreq->close_on_complete;
+
                 qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG,
                        "[C%"PRIu64"][L%"PRIu64"] HTTP request message msg-id=%"PRIu64" encoding complete",
                        hconn->conn_id, link->identity, hreq->base.msg_id);


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[qpid-dispatch] 03/09: DISPATCH-1880: avoid discarding invalid messages on outbound link

kgiusti
In reply to this post by kgiusti
This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git

commit d1966475433ce218bb700bc5d2763f5437868c7f
Author: Kenneth Giusti <[hidden email]>
AuthorDate: Sat Dec 12 11:51:19 2020 -0500

    DISPATCH-1880: avoid discarding invalid messages on outbound link
---
 src/adaptors/http1/http1_client.c | 5 -----
 src/adaptors/http1/http1_server.c | 6 ------
 src/router_node.c                 | 2 +-
 3 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/src/adaptors/http1/http1_client.c b/src/adaptors/http1/http1_client.c
index 952c992..19a70a2 100644
--- a/src/adaptors/http1/http1_client.c
+++ b/src/adaptors/http1/http1_client.c
@@ -1456,8 +1456,6 @@ uint64_t qdr_http1_client_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
                                             bool                    settled)
 {
     qd_message_t        *msg = qdr_delivery_message(delivery);
-    if (qd_message_is_discard(msg))
-        return 0;
 
     _client_request_t  *hreq = (_client_request_t*) qdr_delivery_get_context(delivery);
     if (!hreq) {
@@ -1471,7 +1469,6 @@ uint64_t qdr_http1_client_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
                    "[C%"PRIu64"][L%"PRIu64"] Malformed HTTP/1.x message",
                    hconn->conn_id, link->identity);
             qd_message_set_send_complete(msg);
-            qd_message_set_discard(msg, true);
             qdr_http1_close_connection(hconn, "Malformed response message");
             return PN_REJECTED;
 
@@ -1483,7 +1480,6 @@ uint64_t qdr_http1_client_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
                 qd_log(qdr_http1_adaptor->log, QD_LOG_WARNING,
                        "[C%"PRIu64"][L%"PRIu64"] Discarding malformed message.", hconn->conn_id, link->identity);
                 qd_message_set_send_complete(msg);
-                qd_message_set_discard(msg, true);
                 qdr_http1_close_connection(hconn, "Cannot correlate response message");
                 return PN_REJECTED;
             }
@@ -1545,7 +1541,6 @@ uint64_t qdr_http1_client_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
             } else {
                 // The response was bad.  There's not much that can be done to
                 // recover, so for now I punt...
-                qd_message_set_discard(msg, true);
 
                 // returning a terminal disposition will cause the delivery to be updated and settled,
                 // so drop our reference
diff --git a/src/adaptors/http1/http1_server.c b/src/adaptors/http1/http1_server.c
index 8f03fe7..18a114a 100644
--- a/src/adaptors/http1/http1_server.c
+++ b/src/adaptors/http1/http1_server.c
@@ -631,7 +631,6 @@ static bool _process_requests(qdr_http1_connection_t *hconn)
 
         // clean up the request message delivery
         if (hreq->request_dlv) {
-            qd_message_set_discard(qdr_delivery_message(hreq->request_dlv), true);
 
             if ((!hreq->request_acked || !hreq->request_settled) &&
                 hconn->cfg.aggregation == QD_AGGREGATION_NONE) {
@@ -1407,8 +1406,6 @@ uint64_t qdr_http1_server_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
                                             bool                    settled)
 {
     qd_message_t *msg = qdr_delivery_message(delivery);
-    if (qd_message_is_discard(msg))
-        return 0;
 
     _server_request_t *hreq = (_server_request_t*) qdr_delivery_get_context(delivery);
     if (!hreq) {
@@ -1422,7 +1419,6 @@ uint64_t qdr_http1_server_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
                    "[C%"PRIu64"][L%"PRIu64"] Malformed HTTP/1.x message",
                    hconn->conn_id, link->identity);
             qd_message_set_send_complete(msg);
-            qd_message_set_discard(msg, true);
             qdr_link_flow(qdr_http1_adaptor->core, link, 1, false);
             return PN_REJECTED;
 
@@ -1432,7 +1428,6 @@ uint64_t qdr_http1_server_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
                 qd_log(qdr_http1_adaptor->log, QD_LOG_WARNING,
                        "[C%"PRIu64"][L%"PRIu64"] Discarding malformed message.", hconn->conn_id, link->identity);
                 qd_message_set_send_complete(msg);
-                qd_message_set_discard(msg, true);
                 qdr_link_flow(qdr_http1_adaptor->core, link, 1, false);
                 return PN_REJECTED;
             }
@@ -1458,7 +1453,6 @@ uint64_t qdr_http1_server_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
                        hconn->conn_id, link->identity, hreq->base.msg_id);
             } else {
                 // message invalid
-                qd_message_set_discard(msg, true);
                 _cancel_request(hreq);
 
                 // returning a terminal disposition will cause the delivery to be updated and settled,
diff --git a/src/router_node.c b/src/router_node.c
index f39a4e0..a61fe87 100644
--- a/src/router_node.c
+++ b/src/router_node.c
@@ -1960,7 +1960,7 @@ static void CORE_delivery_update(void *context, qdr_delivery_t *dlv, uint64_t di
             qdr_node_disconnect_deliveries(router->router_core, link, dlv, pnd);
             pn_delivery_settle(pnd);
         } else {
-            if (disp == PN_RELEASED || disp == PN_MODIFIED || qdr_delivery_presettled(dlv)) {
+            if (disp == PN_RELEASED || disp == PN_MODIFIED || disp == PN_REJECTED || qdr_delivery_presettled(dlv)) {
                 //
                 // If the delivery is settled and it is still arriving, defer the settlement
                 // until the content has fully arrived. For now set the disposition on the qdr_delivery


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[qpid-dispatch] 04/09: DISPATCH-1744: bugfix - do not call request_complete if close expected

kgiusti
In reply to this post by kgiusti
This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git

commit 493470537d35ac01e4b537d997bfc7ddd54d279c
Author: Kenneth Giusti <[hidden email]>
AuthorDate: Wed Dec 16 11:09:52 2020 -0500

    DISPATCH-1744: bugfix - do not call request_complete if close expected
---
 include/qpid/dispatch/http1_codec.h | 8 ++++----
 src/adaptors/http1/http1_codec.c    | 8 +++++---
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/qpid/dispatch/http1_codec.h b/include/qpid/dispatch/http1_codec.h
index 79dfcbf..8e53463 100644
--- a/include/qpid/dispatch/http1_codec.h
+++ b/include/qpid/dispatch/http1_codec.h
@@ -238,10 +238,10 @@ int h1_codec_tx_body_str(h1_codec_request_state_t *hrs, char *data);
 // outgoing message construction complete.  The request_complete() callback MAY
 // occur during this call.
 //
-// need_close: set to true if the message is a response that does not provide
-// an explict body length. If true it is up to the caller to close the
-// underlying socket connection after all outgoing data for this request has
-// been sent.
+// need_close: set to true if the outgoing message is an HTTP response that
+// does not provide an explict body length. If true it is up to the caller to
+// close the underlying socket connection after all outgoing data for this
+// request has been sent.
 //
 int h1_codec_tx_done(h1_codec_request_state_t *hrs, bool *need_close);
 
diff --git a/src/adaptors/http1/http1_codec.c b/src/adaptors/http1/http1_codec.c
index ffa8a9f..1047fcc 100644
--- a/src/adaptors/http1/http1_codec.c
+++ b/src/adaptors/http1/http1_codec.c
@@ -1691,9 +1691,11 @@ int h1_codec_tx_done(h1_codec_request_state_t *hrs, bool *need_close)
 
     encoder_reset(encoder);
 
-    if (hrs->request_complete && hrs->response_complete) {
-        conn->config.request_complete(hrs, false);
-        h1_codec_request_state_free(hrs);
+    if (!hrs->close_expected) {
+        if (hrs->request_complete && hrs->response_complete) {
+            conn->config.request_complete(hrs, false);
+            h1_codec_request_state_free(hrs);
+        }
     }
 
     return 0;


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[qpid-dispatch] 05/09: NO-JIRA: clarify delivery inc/decref messages

kgiusti
In reply to this post by kgiusti
This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git

commit 45198bf94969e7c41b8439437c92e99f4ba00a98
Author: Kenneth Giusti <[hidden email]>
AuthorDate: Sun Jan 3 15:10:23 2021 -0500

    NO-JIRA: clarify delivery inc/decref messages
---
 src/adaptors/http1/http1_client.c |  8 ++++----
 src/adaptors/http1/http1_server.c | 12 ++++++------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/adaptors/http1/http1_client.c b/src/adaptors/http1/http1_client.c
index 19a70a2..3c58f3b 100644
--- a/src/adaptors/http1/http1_client.c
+++ b/src/adaptors/http1/http1_client.c
@@ -1490,7 +1490,7 @@ uint64_t qdr_http1_client_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
             rmsg->dlv = delivery;
             DEQ_INIT(rmsg->out_data.fifo);
             qdr_delivery_set_context(delivery, hreq);
-            qdr_delivery_incref(delivery, "referenced by HTTP1 adaptor");
+            qdr_delivery_incref(delivery, "HTTP1 client referencing response delivery");
             DEQ_INSERT_TAIL(hreq->responses, rmsg);
             qd_log(qdr_http1_adaptor->log, QD_LOG_TRACE,
                    "[C%"PRIu64"][L%"PRIu64"] HTTP received response for msg-id=%"PRIu64,
@@ -1545,7 +1545,7 @@ uint64_t qdr_http1_client_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
                 // returning a terminal disposition will cause the delivery to be updated and settled,
                 // so drop our reference
                 qdr_delivery_set_context(rmsg->dlv, 0);
-                qdr_delivery_decref(qdr_http1_adaptor->core, rmsg->dlv, "malformed HTTP1 response, delivery released");
+                qdr_delivery_decref(qdr_http1_adaptor->core, rmsg->dlv, "HTTP1 client releasing malformed response delivery");
                 rmsg->dlv = 0;
                 qdr_http1_close_connection(hconn, "Cannot parse response message");
                 return rmsg->dispo;
@@ -1569,7 +1569,7 @@ static void _client_response_msg_free(_client_request_t *req, _client_response_m
     DEQ_REMOVE(req->responses, rmsg);
     if (rmsg->dlv) {
         qdr_delivery_set_context(rmsg->dlv, 0);
-        qdr_delivery_decref(qdr_http1_adaptor->core, rmsg->dlv, "HTTP1 adaptor response settled");
+        qdr_delivery_decref(qdr_http1_adaptor->core, rmsg->dlv, "HTTP1 client response delivery settled");
     }
 
     qdr_http1_out_data_fifo_cleanup(&rmsg->out_data);
@@ -1602,7 +1602,7 @@ static void _client_request_free(_client_request_t *hreq)
         qd_message_free(hreq->request_msg);
         if (hreq->request_dlv) {
             qdr_delivery_set_context(hreq->request_dlv, 0);
-            qdr_delivery_decref(qdr_http1_adaptor->core, hreq->request_dlv, "HTTP1 adaptor request settled");
+            qdr_delivery_decref(qdr_http1_adaptor->core, hreq->request_dlv, "HTTP1 client request delivery settled");
         }
         qd_compose_free(hreq->request_props);
 
diff --git a/src/adaptors/http1/http1_server.c b/src/adaptors/http1/http1_server.c
index 18a114a..1a661e6 100644
--- a/src/adaptors/http1/http1_server.c
+++ b/src/adaptors/http1/http1_server.c
@@ -649,7 +649,7 @@ static bool _process_requests(qdr_http1_connection_t *hconn)
                 hreq->request_acked = hreq->request_settled = true;
             }
             qdr_delivery_set_context(hreq->request_dlv, 0);
-            qdr_delivery_decref(qdr_http1_adaptor->core, hreq->request_dlv, "HTTP1 adaptor request cancelled");
+            qdr_delivery_decref(qdr_http1_adaptor->core, hreq->request_dlv, "HTTP1 server request cancelled releasing delivery");
             hreq->request_dlv = 0;
         }
 
@@ -713,7 +713,7 @@ static bool _process_requests(qdr_http1_connection_t *hconn)
             hreq->request_acked = true;
             if (hreq->request_settled) {
                 qdr_delivery_set_context(hreq->request_dlv, 0);
-                qdr_delivery_decref(qdr_http1_adaptor->core, hreq->request_dlv, "HTTP1 adaptor request settled");
+                qdr_delivery_decref(qdr_http1_adaptor->core, hreq->request_dlv, "HTTP1 server request settled releasing delivery");
                 hreq->request_dlv = 0;
             }
         }
@@ -1434,7 +1434,7 @@ uint64_t qdr_http1_server_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
 
             hreq->request_dlv = delivery;
             qdr_delivery_set_context(delivery, (void*) hreq);
-            qdr_delivery_incref(delivery, "referenced by HTTP1 adaptor");
+            qdr_delivery_incref(delivery, "HTTP1 server referencing request delivery");
             break;
         }
     }
@@ -1458,7 +1458,7 @@ uint64_t qdr_http1_server_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
                 // returning a terminal disposition will cause the delivery to be updated and settled,
                 // so drop our reference
                 qdr_delivery_set_context(hreq->request_dlv, 0);
-                qdr_delivery_decref(qdr_http1_adaptor->core, hreq->request_dlv, "malformed HTTP1 request, delivery released");
+                qdr_delivery_decref(qdr_http1_adaptor->core, hreq->request_dlv, "HTTP1 server releasing malformed HTTP1 request delivery");
                 hreq->request_dlv = 0;
                 hreq->request_acked = hreq->request_settled = true;
                 return hreq->request_dispo;
@@ -1483,7 +1483,7 @@ static void _server_response_msg_free(_server_request_t *hreq, _server_response_
     qd_compose_free(rmsg->msg_props);
     if (rmsg->dlv) {
         qdr_delivery_set_context(rmsg->dlv, 0);
-        qdr_delivery_decref(qdr_http1_adaptor->core, rmsg->dlv, "HTTP1 adaptor response freed");
+        qdr_delivery_decref(qdr_http1_adaptor->core, rmsg->dlv, "HTTP1 server releasing response delivery");
     }
     free__server_response_msg_t(rmsg);
 }
@@ -1497,7 +1497,7 @@ static void _server_request_free(_server_request_t *hreq)
         qdr_http1_request_base_cleanup(&hreq->base);
         if (hreq->request_dlv) {
             qdr_delivery_set_context(hreq->request_dlv, 0);
-            qdr_delivery_decref(qdr_http1_adaptor->core, hreq->request_dlv, "HTTP1 adaptor request freed");
+            qdr_delivery_decref(qdr_http1_adaptor->core, hreq->request_dlv, "HTTP1 server releasing request delivery");
         }
 
         qdr_http1_out_data_fifo_cleanup(&hreq->out_data);


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[qpid-dispatch] 06/09: DISPATCH-1744: clear close expected flag when close occurs

kgiusti
In reply to this post by kgiusti
This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git

commit 7515af4791c86d1527f23c9d9341969d3c3b0409
Author: Kenneth Giusti <[hidden email]>
AuthorDate: Sun Jan 3 15:25:45 2021 -0500

    DISPATCH-1744: clear close expected flag when close occurs
---
 src/adaptors/http1/http1_codec.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/adaptors/http1/http1_codec.c b/src/adaptors/http1/http1_codec.c
index 1047fcc..7669d26 100644
--- a/src/adaptors/http1/http1_codec.c
+++ b/src/adaptors/http1/http1_codec.c
@@ -1367,12 +1367,14 @@ void h1_codec_connection_rx_closed(h1_codec_connection_t *conn)
         qd_buffer_list_free_buffers(&conn->decoder.incoming);
         decoder->read_ptr = NULL_I_PTR;
 
-        // complete any "done" requests
+        // check if current request is completed
         hrs = DEQ_HEAD(conn->hrs_queue);
-        while (hrs && hrs->response_complete && hrs->request_complete) {
-            conn->config.request_complete(hrs, false);
-            h1_codec_request_state_free(hrs);
-            hrs = DEQ_HEAD(conn->hrs_queue);
+        if (hrs) {
+            hrs->close_expected = false;   // the close just occurred
+            if (hrs->response_complete && hrs->request_complete) {
+                conn->config.request_complete(hrs, false);
+                h1_codec_request_state_free(hrs);
+            }
         }
     }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[qpid-dispatch] 07/09: DISPATCH-1744: discard outgoing request on raw conn close

kgiusti
In reply to this post by kgiusti
This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git

commit 125b7d5275d80023930cfd7cc962b5dbc18a717b
Author: Kenneth Giusti <[hidden email]>
AuthorDate: Sun Jan 3 16:29:45 2021 -0500

    DISPATCH-1744: discard outgoing request on raw conn close
---
 src/adaptors/http1/http1_server.c | 64 +++++++++++++++++++++++++++------------
 1 file changed, 44 insertions(+), 20 deletions(-)

diff --git a/src/adaptors/http1/http1_server.c b/src/adaptors/http1/http1_server.c
index 1a661e6..c8adc62 100644
--- a/src/adaptors/http1/http1_server.c
+++ b/src/adaptors/http1/http1_server.c
@@ -70,6 +70,7 @@ typedef struct _server_request_t {
     uint64_t        request_dispo;   // set by adaptor during encode
     bool            request_settled; // set by adaptor
     bool            request_acked;   // true if dispo sent to core
+    bool            request_discard; // drop incoming request data
     bool            headers_encoded; // True when header encode done
 
     qdr_http1_out_data_fifo_t out_data;  // encoded request written to raw conn
@@ -279,6 +280,14 @@ void qd_http1_delete_connector(qd_dispatch_t *ignored, qd_http_connector_t *ct)
 ////////////////////////////////////////////////////////
 
 
+// Is the hreq currently in flight to the server?
+//
+static inline bool _is_request_in_progress(const _server_request_t *hreq)
+{
+    return hreq && (hreq->base.out_http1_octets > 0 || hreq->cancelled);
+}
+
+
 // Create the qdr links and HTTP codec when the server connection comes up.
 // These links & codec will persist across temporary drops in the connection to
 // the server (like when closing the connection to indicate end of response
@@ -487,15 +496,11 @@ static void _handle_connection_events(pn_event_t *e, qd_server_t *qd_server, voi
     }
 
     case PN_RAW_CONNECTION_CLOSED_WRITE: {
-        // cancel the current request if the request has not been fully written
-        // to the raw connection
+        // discard any remaining outgoing request message data
         _server_request_t *hreq = (_server_request_t*) DEQ_HEAD(hconn->requests);
-        if (hreq) {
-            if (hreq->base.out_http1_octets > 0) {  // req msg written to server
-                if (!DEQ_IS_EMPTY(hreq->out_data.fifo)) {
-                    _cancel_request(hreq);
-                }
-            }
+        if (_is_request_in_progress(hreq)) {
+            hreq->request_discard = true;
+            qdr_http1_out_data_fifo_cleanup(&hreq->out_data);
         }
         pn_raw_connection_close(hconn->raw_conn);
         break;
@@ -504,6 +509,17 @@ static void _handle_connection_events(pn_event_t *e, qd_server_t *qd_server, voi
         qd_log(log, QD_LOG_INFO, "[C%"PRIu64"] Connection closed", hconn->conn_id);
 
         pn_raw_connection_set_context(hconn->raw_conn, 0);
+
+        // Check for a request that is in-progress - it needs to be cancelled.
+        // However there is an exception: the server has completed sending a
+        // response message and closed the connection, but the outgoing request
+        // message has not completed (example: a streaming POST that has been
+        // rejected by the server). In this case wait until the request message
+        // has fully arrived from the core.
+
+        _server_request_t *hreq = (_server_request_t*) DEQ_HEAD(hconn->requests);
+        if (_is_request_in_progress(hreq) && !hreq->response_complete)
+            _cancel_request(hreq);
         _process_requests(hconn);
 
         //
@@ -757,12 +773,16 @@ static void _server_tx_buffers_cb(h1_codec_request_state_t *hrs, qd_buffer_list_
     _server_request_t       *hreq = (_server_request_t*) h1_codec_request_state_get_context(hrs);
     qdr_http1_connection_t *hconn = hreq->base.hconn;
 
-    qd_log(qdr_http1_adaptor->log, QD_LOG_TRACE,
-           "[C%"PRIu64"][L%"PRIu64"] Sending %u octets to server",
-           hconn->conn_id, hconn->out_link_id, len);
-    qdr_http1_enqueue_buffer_list(&hreq->out_data, blist);
-    if (hreq == (_server_request_t*) DEQ_HEAD(hconn->requests)) {
-        _write_pending_request(hreq);
+    if (hreq->request_discard)
+        qd_buffer_list_free_buffers(blist);
+    else {
+        qd_log(qdr_http1_adaptor->log, QD_LOG_TRACE,
+               "[C%"PRIu64"][L%"PRIu64"] Sending %u octets to server",
+               hconn->conn_id, hconn->out_link_id, len);
+        qdr_http1_enqueue_buffer_list(&hreq->out_data, blist);
+        if (hreq == (_server_request_t*) DEQ_HEAD(hconn->requests)) {
+            _write_pending_request(hreq);
+        }
     }
 }
 
@@ -774,12 +794,16 @@ static void _server_tx_stream_data_cb(h1_codec_request_state_t *hrs, qd_message_
     _server_request_t       *hreq = (_server_request_t*) h1_codec_request_state_get_context(hrs);
     qdr_http1_connection_t *hconn = hreq->base.hconn;
 
-    qd_log(qdr_http1_adaptor->log, QD_LOG_TRACE,
-           "[C%"PRIu64"][L%"PRIu64"] Sending body data to server",
-           hconn->conn_id, hconn->out_link_id);
-    qdr_http1_enqueue_stream_data(&hreq->out_data, stream_data);
-    if (hreq == (_server_request_t*) DEQ_HEAD(hconn->requests) && hconn->raw_conn) {
-        _write_pending_request(hreq);
+    if (hreq->request_discard)
+        qd_message_stream_data_release(stream_data);
+    else {
+        qd_log(qdr_http1_adaptor->log, QD_LOG_TRACE,
+               "[C%"PRIu64"][L%"PRIu64"] Sending body data to server",
+               hconn->conn_id, hconn->out_link_id);
+        qdr_http1_enqueue_stream_data(&hreq->out_data, stream_data);
+        if (hreq == (_server_request_t*) DEQ_HEAD(hconn->requests)) {
+            _write_pending_request(hreq);
+        }
     }
 }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[qpid-dispatch] 08/09: DISPATCH-1744: re-factor how requests are retired

kgiusti
In reply to this post by kgiusti
This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git

commit 806188101578c3d7d5cb1b78b115d15a43784d90
Author: Kenneth Giusti <[hidden email]>
AuthorDate: Sun Jan 3 16:39:18 2021 -0500

    DISPATCH-1744: re-factor how requests are retired
   
    This patch makes the process of finishing up the HTTP request more
    synchronous by preventing the core from pushing the next outgoing
    request delivery until after the current request is completed.
---
 src/adaptors/http1/http1_server.c | 239 +++++++++++++++++++-------------------
 1 file changed, 119 insertions(+), 120 deletions(-)

diff --git a/src/adaptors/http1/http1_server.c b/src/adaptors/http1/http1_server.c
index c8adc62..1e062c6 100644
--- a/src/adaptors/http1/http1_server.c
+++ b/src/adaptors/http1/http1_server.c
@@ -127,7 +127,9 @@ static void _server_response_msg_free(_server_request_t *req, _server_response_m
 static void _server_request_free(_server_request_t *hreq);
 static void _write_pending_request(_server_request_t *req);
 static void _cancel_request(_server_request_t *req);
-static bool _process_requests(qdr_http1_connection_t *hconn);
+static bool _process_request(_server_request_t *req);
+static void _encode_request_message(_server_request_t *hreq);
+static void _send_request_message(_server_request_t *hreq);
 
 
 ////////////////////////////////////////////////////////
@@ -424,20 +426,27 @@ static void _do_reconnect(void *context)
             return;
         }
 
-        _process_requests(hconn);
+        _process_request((_server_request_t*) DEQ_HEAD(hconn->requests));
     }
 
-    // lock out core activation
-    sys_mutex_lock(qdr_http1_adaptor->lock);
-    hconn->raw_conn = pn_raw_connection();
-    pn_raw_connection_set_context(hconn->raw_conn, &hconn->handler_context);
-    // this next call may immediately reschedule the connection on another I/O
-    // thread. After this call hconn may no longer be valid!
-    pn_proactor_raw_connect(qd_server_proactor(hconn->qd_server), hconn->raw_conn, hconn->cfg.host_port);
-    sys_mutex_unlock(qdr_http1_adaptor->lock);
+    // Do not attempt to re-connect if the current request is still in
+    // progress. This happens when the server has closed the connection before
+    // the request message has fully arrived (!rx_complete).
+    // qdr_connection_process() will continue to invoke the
+    // qdr_http1_server_core_link_deliver callback until the request message is
+    // complete.
 
-    qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG,
-           "[C%"PRIu64"] Connecting to HTTP server...", conn_id);
+    if (!_is_request_in_progress((_server_request_t*) DEQ_HEAD(hconn->requests))) {
+        qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG,
+               "[C%"PRIu64"] Connecting to HTTP server...", conn_id);
+        sys_mutex_lock(qdr_http1_adaptor->lock);
+        hconn->raw_conn = pn_raw_connection();
+        pn_raw_connection_set_context(hconn->raw_conn, &hconn->handler_context);
+        // this next call may immediately reschedule the connection on another I/O
+        // thread. After this call hconn may no longer be valid!
+        pn_proactor_raw_connect(qd_server_proactor(hconn->qd_server), hconn->raw_conn, hconn->cfg.host_port);
+        sys_mutex_unlock(qdr_http1_adaptor->lock);
+    }
 }
 
 static void _accept_and_settle_request(_server_request_t *hreq)
@@ -480,17 +489,6 @@ static void _handle_connection_events(pn_event_t *e, qd_server_t *qd_server, voi
         // notify the codec so it can complete the current response
         // message (response body terminated on connection closed)
         h1_codec_connection_rx_closed(hconn->http_conn);
-
-        // if the response for the current request has not fully arrived cancel
-        // the request
-        _server_request_t *hreq = (_server_request_t*) DEQ_HEAD(hconn->requests);
-        if (hreq) {
-            if (hreq->base.out_http1_octets > 0) {  // req msg written to server
-                if (!hreq->response_complete) {
-                    _cancel_request(hreq);
-                }
-            }
-        }
         pn_raw_connection_close(hconn->raw_conn);
         break;
     }
@@ -520,12 +518,13 @@ static void _handle_connection_events(pn_event_t *e, qd_server_t *qd_server, voi
         _server_request_t *hreq = (_server_request_t*) DEQ_HEAD(hconn->requests);
         if (_is_request_in_progress(hreq) && !hreq->response_complete)
             _cancel_request(hreq);
-        _process_requests(hconn);
+        _process_request(hreq);
 
         //
-        // reconnect to the server. Leave the links intact so pending requests
-        // are not aborted.  If we fail to reconnect after LINK_TIMEOUT_MSECS
-        // drop the links to prevent additional request from arriving.
+        // Try to reconnect to the server. Leave the links intact so pending
+        // requests are not aborted.  If we fail to reconnect after
+        // LINK_TIMEOUT_MSECS drop the links to prevent additional request from
+        // arriving.
         //
 
         bool reconnect = false;
@@ -554,12 +553,10 @@ static void _handle_connection_events(pn_event_t *e, qd_server_t *qd_server, voi
         return;
     }
     case PN_RAW_CONNECTION_NEED_WRITE_BUFFERS: {
-        qd_log(log, QD_LOG_DEBUG, "[C%"PRIu64"] Need write buffers", hconn->conn_id);
-        _write_pending_request((_server_request_t*) DEQ_HEAD(hconn->requests));
+        _send_request_message((_server_request_t*) DEQ_HEAD(hconn->requests));
         break;
     }
     case PN_RAW_CONNECTION_NEED_READ_BUFFERS: {
-        qd_log(log, QD_LOG_DEBUG, "[C%"PRIu64"] Need read buffers", hconn->conn_id);
         // @TODO(kgiusti): backpressure if no credit
         // if (hconn->in_link_credit > 0 */)
         int granted = qda_raw_conn_grant_read_buffers(hconn->raw_conn);
@@ -614,8 +611,7 @@ static void _handle_connection_events(pn_event_t *e, qd_server_t *qd_server, voi
         qdr_http1_connection_free(hconn);
 
     } else {
-        bool need_close = _process_requests(hconn);
-
+        bool need_close = _process_request((_server_request_t*) DEQ_HEAD(hconn->requests));
         if (need_close) {
             qd_log(log, QD_LOG_DEBUG, "[C%"PRIu64"] Closing connection!", hconn->conn_id);
             qdr_http1_close_connection(hconn, "HTTP Request requires connection close");
@@ -624,27 +620,26 @@ static void _handle_connection_events(pn_event_t *e, qd_server_t *qd_server, voi
 }
 
 
-// See if the current request can be completed and the next pending request
-// started. Return true if the connection must be closed before starting the
-// next request.
-static bool _process_requests(qdr_http1_connection_t *hconn)
+// Check the head request for completion. Return true if the connection must be
+// closed before starting the next request.
+static bool _process_request(_server_request_t *hreq)
 {
-    bool              need_close = false;
-    _server_request_t *next_hreq = 0;
-    _server_request_t      *hreq = (_server_request_t*) DEQ_HEAD(hconn->requests);
+    bool need_close = false;
 
     if (!hreq)
         return need_close;
 
-    qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG,
-           "[C%"PRIu64"] Processing current HTTP request msg-id=%"PRIu64", state=%s",
-           hconn->conn_id, hreq->base.msg_id,
-           hreq->codec_completed ? "codec complete"
-           : hreq->cancelled ? "request cancelled"
-           : "in-progress");
+    assert(DEQ_PREV(&hreq->base) == 0);  // preserve order!
+
+    qdr_http1_connection_t *hconn = hreq->base.hconn;
 
     if (hreq->cancelled) {
 
+        // have to wait until all buffers returned from proton
+        // before we can release the request
+        if (qdr_http1_out_data_buffers_outstanding(&hreq->out_data))
+            return false;
+
         // clean up the request message delivery
         if (hreq->request_dlv) {
 
@@ -655,6 +650,8 @@ static bool _process_requests(qdr_http1_connection_t *hconn)
                     hreq->request_dispo = (hreq->base.out_http1_octets > 0
                                            ? PN_MODIFIED : PN_RELEASED);
 
+                qd_message_set_send_complete(qdr_delivery_message(hreq->request_dlv));
+                qdr_link_complete_sent_message(qdr_http1_adaptor->core, hconn->out_link);
                 qdr_delivery_remote_state_updated(qdr_http1_adaptor->core,
                                                   hreq->request_dlv,
                                                   hreq->request_dispo,
@@ -680,19 +677,12 @@ static bool _process_requests(qdr_http1_connection_t *hconn)
             rmsg = DEQ_HEAD(hreq->responses);
         }
 
-        // have to wait until all buffers returned from proton
-        // before we can release the request
-        if (qdr_http1_out_data_buffers_outstanding(&hreq->out_data))
-            return false;
-
         // it is safe to keep the connection up if this request has never been
         // written to the connection, otherwise the state of the connection is
         // unknown so close it
 
         if (hreq->base.out_http1_octets > 0)
             need_close = true;
-        else
-            next_hreq = (_server_request_t*) DEQ_NEXT(&hreq->base);
 
         qd_log(qdr_http1_adaptor->log, QD_LOG_TRACE, "[C%"PRIu64"] HTTP request msg-id=%"PRIu64" cancelled",
                hconn->conn_id, hreq->base.msg_id);
@@ -701,7 +691,6 @@ static bool _process_requests(qdr_http1_connection_t *hconn)
         if (hconn->out_link)
             qdr_link_flow(qdr_http1_adaptor->core, hconn->out_link, 1, false);
 
-
     } else if (hreq->codec_completed) {
 
         // The request message has been fully encoded and the response msg(s)
@@ -719,6 +708,11 @@ static bool _process_requests(qdr_http1_connection_t *hconn)
             assert(hreq->request_dlv);
             assert(hreq->request_dispo == PN_ACCEPTED);
             hreq->request_settled = DEQ_IS_EMPTY(hreq->responses);
+
+            if (!hreq->request_acked) {
+                qd_message_set_send_complete(qdr_delivery_message(hreq->request_dlv));
+                qdr_link_complete_sent_message(qdr_http1_adaptor->core, hconn->out_link);
+            }
             qdr_delivery_remote_state_updated(qdr_http1_adaptor->core,
                                               hreq->request_dlv,
                                               hreq->request_dispo,
@@ -737,12 +731,6 @@ static bool _process_requests(qdr_http1_connection_t *hconn)
         if (hreq->request_acked && hreq->request_settled && DEQ_SIZE(hreq->out_data.fifo) == 0) {
             qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG, "[C%"PRIu64"] HTTP request msg-id=%"PRIu64" completed!",
                    hconn->conn_id, hreq->base.msg_id);
-
-            if (hreq->close_on_complete)
-                need_close = true;
-            else
-                next_hreq = (_server_request_t*) DEQ_NEXT(&hreq->base);
-
             _server_request_free(hreq);
 
             if (hconn->out_link)
@@ -750,13 +738,6 @@ static bool _process_requests(qdr_http1_connection_t *hconn)
         }
     }
 
-    if (next_hreq) {
-        qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG,
-               "[C%"PRIu64"] starting new HTTP request msg-id=%"PRIu64,
-               hconn->conn_id, next_hreq->base.msg_id);
-        _write_pending_request(next_hreq);
-    }
-
     return need_close;
 }
 
@@ -1357,28 +1338,28 @@ exit:
 }
 
 
-// Encode an outbound AMQP message as an HTTP Request.  Returns PN_ACCEPTED
-// when complete, 0 if incomplete and PN_REJECTED if encoding error.
+// Encode an outbound AMQP message as an HTTP Request.  Sets the request_dispo
+// when the encoding completes either successfully or in error.
 //
-static uint64_t _encode_request_message(_server_request_t *hreq)
+static void _encode_request_message(_server_request_t *hreq)
 {
     qdr_http1_connection_t    *hconn = hreq->base.hconn;
     qd_message_t                *msg = qdr_delivery_message(hreq->request_dlv);
 
     if (!hreq->headers_encoded) {
-        uint64_t outcome = _send_request_headers(hreq, msg);
+        hreq->request_dispo = _send_request_headers(hreq, msg);
         hreq->headers_encoded = true;
-        if (outcome) {
+        if (hreq->request_dispo) {
             qd_log(qdr_http1_adaptor->log, QD_LOG_WARNING,
                    "[C%"PRIu64"][L%"PRIu64"] Rejecting malformed message msg-id=%"PRIu64,
                    hconn->conn_id, hconn->out_link_id, hreq->base.msg_id);
-            return outcome;
+            return;
         }
     }
 
-    qd_message_stream_data_t *stream_data = 0;
+    while (hreq->request_dispo == 0) {
 
-    while (true) {
+        qd_message_stream_data_t *stream_data = 0;
         switch (qd_message_next_stream_data(msg, &stream_data)) {
         case QD_MESSAGE_STREAM_DATA_BODY_OK: {
 
@@ -1390,7 +1371,7 @@ static uint64_t _encode_request_message(_server_request_t *hreq)
                 qd_log(qdr_http1_adaptor->log, QD_LOG_WARNING,
                        "[C%"PRIu64"][L%"PRIu64"] body data encode failed",
                        hconn->conn_id, hconn->out_link_id);
-                return PN_REJECTED;
+                hreq->request_dispo = PN_REJECTED;
             }
             break;
         }
@@ -1401,20 +1382,58 @@ static uint64_t _encode_request_message(_server_request_t *hreq)
 
         case QD_MESSAGE_STREAM_DATA_NO_MORE:
             // indicate this message is complete
-            return PN_ACCEPTED;
+            qd_log(hconn->adaptor->log, QD_LOG_TRACE,
+                   "[C%"PRIu64"][L%"PRIu64"] Request %p body data encode complete",
+                   hconn->conn_id, hconn->out_link_id, (void*) hreq);
+            hreq->request_dispo = PN_ACCEPTED;
+            break;
 
         case QD_MESSAGE_STREAM_DATA_INCOMPLETE:
-            qd_log(qdr_http1_adaptor->log, QD_LOG_TRACE,
-                   "[C%"PRIu64"][L%"PRIu64"] body data need more",
-                   hconn->conn_id, hconn->out_link_id);
-            return 0;  // wait for more
+            return;  // wait for more
 
         case QD_MESSAGE_STREAM_DATA_INVALID:
             qd_log(qdr_http1_adaptor->log, QD_LOG_WARNING,
                    "[C%"PRIu64"][L%"PRIu64"] Rejecting corrupted body data.",
                    hconn->conn_id, hconn->out_link_id);
-            return PN_REJECTED;
+            hreq->request_dispo = PN_REJECTED;
+            break;
+        }
+    }
+}
+
+
+// encode the request message and write it out to the server.
+static void _send_request_message(_server_request_t *hreq)
+{
+    if (hreq) {
+        assert(DEQ_PREV(&hreq->base) == 0);  // preserve order!
+        qdr_http1_connection_t *hconn = hreq->base.hconn;
+        if (hreq->request_dispo == 0) {
+            _encode_request_message(hreq);
+            switch (hreq->request_dispo) {
+
+            case 0:
+                // streaming, not complete
+                break;
+
+            case PN_ACCEPTED: {
+                // completed successfully
+                bool ignore = false;  // used client-facing only
+                h1_codec_tx_done(hreq->base.lib_rs, &ignore);
+                qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG,
+                       "[C%"PRIu64"][L%"PRIu64"] HTTP %p request message msg-id=%"PRIu64" encoding complete",
+                       hconn->conn_id, hconn->out_link_id, (void*)hreq, hreq->base.msg_id);
+                break;
+            }
+
+            default:
+                // encoding failure
+                _cancel_request(hreq);
+                return;
+            }
         }
+        // write encoded data to raw conn
+        _write_pending_request(hreq);
     }
 }
 
@@ -1463,32 +1482,8 @@ uint64_t qdr_http1_server_core_link_deliver(qdr_http1_adaptor_t    *adaptor,
         }
     }
 
-    if (!hreq->request_dispo) {
-        hreq->request_dispo = _encode_request_message(hreq);
-        if (hreq->request_dispo) {
-            qd_message_set_send_complete(msg);
-            if (hreq->request_dispo == PN_ACCEPTED) {
-                bool need_close = false;
-                h1_codec_tx_done(hreq->base.lib_rs, &need_close);
-                hreq->close_on_complete = need_close || hreq->close_on_complete;
-
-                qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG,
-                       "[C%"PRIu64"][L%"PRIu64"] HTTP request message msg-id=%"PRIu64" encoding complete",
-                       hconn->conn_id, link->identity, hreq->base.msg_id);
-            } else {
-                // message invalid
-                _cancel_request(hreq);
-
-                // returning a terminal disposition will cause the delivery to be updated and settled,
-                // so drop our reference
-                qdr_delivery_set_context(hreq->request_dlv, 0);
-                qdr_delivery_decref(qdr_http1_adaptor->core, hreq->request_dlv, "HTTP1 server releasing malformed HTTP1 request delivery");
-                hreq->request_dlv = 0;
-                hreq->request_acked = hreq->request_settled = true;
-                return hreq->request_dispo;
-            }
-        }
-    }
+    if (DEQ_HEAD(hconn->requests) == &hreq->base)
+        _send_request_message(hreq);
 
     return 0;
 }
@@ -1544,8 +1539,9 @@ static void _write_pending_request(_server_request_t *hreq)
         uint64_t written = qdr_http1_write_out_data(hreq->base.hconn, &hreq->out_data);
         hreq->base.out_http1_octets += written;
         if (written)
-            qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG, "[C%"PRIu64"] %"PRIu64" octets written",
-                   hreq->base.hconn->conn_id, written);
+            qd_log(qdr_http1_adaptor->log, QD_LOG_TRACE,
+                   "[C%"PRIu64"][L%"PRIu64"] %"PRIu64" request octets written to server",
+                   hreq->base.hconn->conn_id, hreq->base.hconn->out_link_id, written);
     }
 }
 
@@ -1562,18 +1558,21 @@ void qdr_http1_server_conn_cleanup(qdr_http1_connection_t *hconn)
 
 static void _cancel_request(_server_request_t *hreq)
 {
-    qd_log(qdr_http1_adaptor->log, QD_LOG_TRACE,
-           "[C%"PRIu64"][L%"PRIu64"] Cancelling HTTP Request msg-id=%"PRIu64,
-           hreq->base.hconn->conn_id, hreq->base.hconn->out_link_id,
-           hreq->base.msg_id);
+    if (!hreq->cancelled) {
 
-    if (!hreq->base.lib_rs) {
-        // never even got to encoding it - manually mark it cancelled
-        hreq->cancelled = true;
-    } else {
-        // cleanup codec state - this will call _server_request_complete_cb()
-        // with cancelled = true
-        h1_codec_request_state_cancel(hreq->base.lib_rs);
+        qd_log(qdr_http1_adaptor->log, QD_LOG_TRACE,
+               "[C%"PRIu64"][L%"PRIu64"] Cancelling HTTP Request msg-id=%"PRIu64,
+               hreq->base.hconn->conn_id, hreq->base.hconn->out_link_id,
+               hreq->base.msg_id);
+
+        if (!hreq->base.lib_rs) {
+            // never even got to encoding it - manually mark it cancelled
+            hreq->cancelled = true;
+        } else {
+            // cleanup codec state - this will call _server_request_complete_cb()
+            // with cancelled = true
+            h1_codec_request_state_cancel(hreq->base.lib_rs);
+        }
     }
 
     // cleanup occurs at the end of the connection event handler


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[qpid-dispatch] 09/09: DISPATCH-1744: remove 100 Continue test on HTTP/1.0 server

kgiusti
In reply to this post by kgiusti
This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git

commit a14b289b9df2e99329e8023c31afbc4ee7edf013
Author: Kenneth Giusti <[hidden email]>
AuthorDate: Mon Jan 4 13:29:42 2021 -0500

    DISPATCH-1744: remove 100 Continue test on HTTP/1.0 server
   
    The python HTTPServer will occasionally incorrectly close the TCP
    connection after sending a 100-Continue response.  This causes the
    test to fail.  This failure is not due to the adaptor.
   
    This closes #942
---
 src/adaptors/http1/http1_server.c   | 6 ------
 tests/system_tests_http1_adaptor.py | 8 +++-----
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/src/adaptors/http1/http1_server.c b/src/adaptors/http1/http1_server.c
index 1e062c6..f020f70 100644
--- a/src/adaptors/http1/http1_server.c
+++ b/src/adaptors/http1/http1_server.c
@@ -761,9 +761,6 @@ static void _server_tx_buffers_cb(h1_codec_request_state_t *hrs, qd_buffer_list_
                "[C%"PRIu64"][L%"PRIu64"] Sending %u octets to server",
                hconn->conn_id, hconn->out_link_id, len);
         qdr_http1_enqueue_buffer_list(&hreq->out_data, blist);
-        if (hreq == (_server_request_t*) DEQ_HEAD(hconn->requests)) {
-            _write_pending_request(hreq);
-        }
     }
 }
 
@@ -782,9 +779,6 @@ static void _server_tx_stream_data_cb(h1_codec_request_state_t *hrs, qd_message_
                "[C%"PRIu64"][L%"PRIu64"] Sending body data to server",
                hconn->conn_id, hconn->out_link_id);
         qdr_http1_enqueue_stream_data(&hreq->out_data, stream_data);
-        if (hreq == (_server_request_t*) DEQ_HEAD(hconn->requests)) {
-            _write_pending_request(hreq);
-        }
     }
 }
 
diff --git a/tests/system_tests_http1_adaptor.py b/tests/system_tests_http1_adaptor.py
index 237adca..57979cc 100644
--- a/tests/system_tests_http1_adaptor.py
+++ b/tests/system_tests_http1_adaptor.py
@@ -708,11 +708,9 @@ class Http1AdaptorOneRouterTest(TestCase):
 
             (RequestMsg("GET", "/GET/info_content_len",
                         headers={"Content-Length": 0}),
-             [ResponseMsg(100, reason="Continue",
-                          headers={"Blab": 1, "Blob": "?"}),
-              ResponseMsg(200, reason="OK",
-                          headers={"Content-Type": "text/plain;charset=utf-8"},
-                          body=b'?')],
+             ResponseMsg(200, reason="OK",
+                         headers={"Content-Type": "text/plain;charset=utf-8"},
+                         body=b'?'),
              ResponseValidator(expect_headers={'Content-Type': "text/plain;charset=utf-8"},
                                expect_body=b'?')),
 


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]