0014-BUG-MEDIUM-http-Switch-HTTP-responses-in-TUNNEL-mode.patch 4.8 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118
  1. From f82344c1cf20afcf77e8c3df8f9d341d659da93b Mon Sep 17 00:00:00 2001
  2. From: Christopher Faulet <cfaulet@haproxy.com>
  3. Date: Tue, 18 Jul 2017 11:42:08 +0200
  4. Subject: [PATCH 14/18] BUG/MEDIUM: http: Switch HTTP responses in TUNNEL mode
  5. when body length is undefined
  6. When the body length of a HTTP response is undefined, the HTTP parser is blocked
  7. in the body parsing. Before HAProxy 1.7, in this case, because
  8. AN_RES_HTTP_XFER_BODY is never set, there is no visible effect. When the server
  9. closes its connection to terminate the response, HAProxy catches it as a normal
  10. closure. Since 1.7, we always set this analyzer to enter at least once in
  11. http_response_forward_body. But, in the present case, when the server connection
  12. is closed, http_response_forward_body is called one time too many. The response
  13. is correctly sent to the client, but an error is catched and logged with "SD--"
  14. flags.
  15. To reproduce the bug, you can use the configuration "tests/test-fsm.cfg". The
  16. tests 3 and 21 hit the bug.
  17. Idea to fix the bug is to switch the response in TUNNEL mode without switching
  18. the request. This is possible because of previous patches.
  19. First, we need to detect responses with undefined body length during states
  20. synchronization. Excluding tunnelled transactions, when the response length is
  21. undefined, TX_CON_WANT_CLO is always set on the transaction. So, when states are
  22. synchronized, if TX_CON_WANT_CLO is set, the response is switched in TUNNEL mode
  23. and the request remains unchanged.
  24. Then, in http_msg_forward_body, we add a specific check to switch the response
  25. in DONE mode if the body length is undefined and if there is no data filter.
  26. This patch depends on following previous commits:
  27. * MINOR: http: Switch requests/responses in TUNNEL mode only by checking txn flags
  28. * MINOR: http: Reorder/rewrite checks in http_resync_states
  29. This patch must be backported in 1.7 with 2 previous ones.
  30. (cherry picked from commit 1486b0ab6de744e14ae684af105951345534f9ec)
  31. Signed-off-by: William Lallemand <wlallemand@haproxy.org>
  32. ---
  33. src/proto_http.c | 37 +++++++++++++++++++++++++------------
  34. 1 file changed, 25 insertions(+), 12 deletions(-)
  35. diff --git a/src/proto_http.c b/src/proto_http.c
  36. index 00a92cdb..e776e4d5 100644
  37. --- a/src/proto_http.c
  38. +++ b/src/proto_http.c
  39. @@ -5354,7 +5354,16 @@ int http_sync_req_state(struct stream *s)
  40. * let's enforce it now that we're not expecting any new
  41. * data to come. The caller knows the stream is complete
  42. * once both states are CLOSED.
  43. + *
  44. + * However, there is an exception if the response
  45. + * length is undefined. In this case, we need to wait
  46. + * the close from the server. The response will be
  47. + * switched in TUNNEL mode until the end.
  48. */
  49. + if (!(txn->rsp.flags & HTTP_MSGF_XFER_LEN) &&
  50. + txn->rsp.msg_state != HTTP_MSG_CLOSED)
  51. + goto check_channel_flags;
  52. +
  53. if (!(chn->flags & (CF_SHUTW|CF_SHUTW_NOW))) {
  54. channel_shutr_now(chn);
  55. channel_shutw_now(chn);
  56. @@ -5471,8 +5480,16 @@ int http_sync_res_state(struct stream *s)
  57. * let's enforce it now that we're not expecting any new
  58. * data to come. The caller knows the stream is complete
  59. * once both states are CLOSED.
  60. + *
  61. + * However, there is an exception if the response length
  62. + * is undefined. In this case, we switch in TUNNEL mode.
  63. */
  64. - if (!(chn->flags & (CF_SHUTW|CF_SHUTW_NOW))) {
  65. + if (!(txn->rsp.flags & HTTP_MSGF_XFER_LEN)) {
  66. + channel_auto_read(chn);
  67. + txn->rsp.msg_state = HTTP_MSG_TUNNEL;
  68. + chn->flags |= CF_NEVER_WAIT;
  69. + }
  70. + else if (!(chn->flags & (CF_SHUTW|CF_SHUTW_NOW))) {
  71. channel_shutr_now(chn);
  72. channel_shutw_now(chn);
  73. }
  74. @@ -6952,14 +6969,6 @@ int http_response_forward_body(struct stream *s, struct channel *res, int an_bit
  75. if ((msg->flags & HTTP_MSGF_TE_CHNK) || (msg->flags & HTTP_MSGF_COMPRESSING))
  76. res->flags |= CF_EXPECT_MORE;
  77. - /* If there is neither content-length, nor transfer-encoding header
  78. - * _AND_ there is no data filtering, we can safely forward all data
  79. - * indefinitely. */
  80. - if (!(msg->flags & HTTP_MSGF_XFER_LEN) && !HAS_DATA_FILTERS(s, res)) {
  81. - buffer_flush(res->buf);
  82. - channel_forward_forever(res);
  83. - }
  84. -
  85. /* the stream handler will take care of timeouts and errors */
  86. return 0;
  87. @@ -7036,9 +7045,13 @@ http_msg_forward_body(struct stream *s, struct http_msg *msg)
  88. goto missing_data_or_waiting;
  89. }
  90. - /* The server still sending data that should be filtered */
  91. - if (!(msg->flags & HTTP_MSGF_XFER_LEN) && !(chn->flags & CF_SHUTR))
  92. - goto missing_data_or_waiting;
  93. + /* This check can only be true for a response. HTTP_MSGF_XFER_LEN is
  94. + * always set for a request. */
  95. + if (!(msg->flags & HTTP_MSGF_XFER_LEN)) {
  96. + /* The server still sending data that should be filtered */
  97. + if (!(chn->flags & CF_SHUTR) && HAS_DATA_FILTERS(s, chn))
  98. + goto missing_data_or_waiting;
  99. + }
  100. msg->msg_state = HTTP_MSG_ENDING;
  101. --
  102. 2.13.0