TS-1988 Unscrew cquuc and cquup Here's what happens, and this is the same problem as with cquuh, which I'll fix in a separate commit:
When producing log entries, we call the marshal function twice: first to calculate the final length of the string a second time to actually write the string This two-phased behavior is controlled via the "buf" parameter. With a NULL value, it means to only calculate the length. With the changes made to cquuc/cquup, the lenghts were not properly calculate, which is exactly the problem with cquuh as well. So, it's important to always calculate the correct length, and return that. Which is confusing, since the buf parameter seems to have been in place to make sure we don't write to a NULL buf. Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/ea35372e Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/ea35372e Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/ea35372e Branch: refs/heads/5.0.x Commit: ea35372ef072724783c3c963917ea57790a8adae Parents: 8f13ff5 Author: Leif Hedstrom <zw...@apache.org> Authored: Mon Oct 21 16:53:11 2013 -0600 Committer: Leif Hedstrom <zw...@apache.org> Committed: Mon Oct 21 16:58:01 2013 -0600 ---------------------------------------------------------------------- proxy/logging/LogAccessHttp.cc | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ea35372e/proxy/logging/LogAccessHttp.cc ---------------------------------------------------------------------- diff --git a/proxy/logging/LogAccessHttp.cc b/proxy/logging/LogAccessHttp.cc index b2bfb58..ddc5c7d 100644 --- a/proxy/logging/LogAccessHttp.cc +++ b/proxy/logging/LogAccessHttp.cc @@ -315,18 +315,19 @@ LogAccessHttp::marshal_client_req_unmapped_url_canon(char *buf) { int len = INK_MIN_ALIGN; - if (buf) { - validate_unmapped_url(); - if (0 == m_client_req_unmapped_url_canon_len) { - // If the unmapped URL isn't populated, we'll fall back to the original - // client URL. This helps for example server intercepts to continue to - // log the requests, even when there is no remap rule for it. - len = marshal_client_req_url_canon(buf); - } else { - len = round_strlen(m_client_req_unmapped_url_canon_len + 1); // +1 for eos + validate_unmapped_url(); + if (0 == m_client_req_unmapped_url_canon_len) { + // If the unmapped URL isn't populated, we'll fall back to the original + // client URL. This helps for example server intercepts to continue to + // log the requests, even when there is no remap rule for it. + len = marshal_client_req_url_canon(buf); + } else { + len = round_strlen(m_client_req_unmapped_url_canon_len + 1); // +1 for eos + if (buf) { marshal_mem(buf, m_client_req_unmapped_url_canon_str, m_client_req_unmapped_url_canon_len, len); } } + return len; } @@ -338,13 +339,14 @@ LogAccessHttp::marshal_client_req_unmapped_url_path(char *buf) { int len = INK_MIN_ALIGN; - if (buf) { - validate_unmapped_url(); - validate_unmapped_url_path(); - if (0 == m_client_req_unmapped_url_path_len) { - len = marshal_client_req_url_path(buf); - } else { - len = round_strlen(m_client_req_unmapped_url_path_len + 1); // +1 for eos + validate_unmapped_url(); + validate_unmapped_url_path(); + + if (0 == m_client_req_unmapped_url_path_len) { + len = marshal_client_req_url_path(buf); + } else { + len = round_strlen(m_client_req_unmapped_url_path_len + 1); // +1 for eos + if (buf) { marshal_mem(buf, m_client_req_unmapped_url_path_str, m_client_req_unmapped_url_path_len, len); } }