You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by shinrich <gi...@git.apache.org> on 2016/09/12 15:38:47 UTC

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

GitHub user shinrich opened a pull request:

    https://github.com/apache/trafficserver/pull/1007

    TS-4703: Add API to retrieve client protocols

    Added API's as discussed on the API review (also on the bug).
    
    Added regression test and example plugin to exercise the options.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/shinrich/trafficserver ts-4703

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/trafficserver/pull/1007.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1007
    
----
commit 9888698a880f8cc78812c3e79aba77db710ac451
Author: Susan Hinrichs <sh...@ieee.org>
Date:   2016-09-10T00:22:15Z

    TS-4703: Add API to retrieve client protocols

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78416314
  
    --- Diff: iocore/net/NetVConnection.cc ---
    @@ -43,3 +44,39 @@ NetVConnection::cancel_OOB()
     {
       return;
     }
    +
    +const char *
    +NetVCOptions::get_proto_string() const
    +{
    +  const char *retval;
    +  switch (ip_proto) {
    +  case USE_TCP:
    +    retval = TS_PROTO_TAG_TCP;
    +    break;
    +  case USE_UDP:
    +    retval = TS_PROTO_TAG_UDP;
    +    break;
    +  default:
    +    retval = NULL;
    +    break;
    +  }
    +  return retval;
    +}
    --- End diff --
    
    Or even just init `retval` to `NULL` and drop the `default` case.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78434484
  
    --- Diff: iocore/net/SSLUtils.cc ---
    @@ -2055,8 +2055,12 @@ SSLParseCertificateConfiguration(const SSLConfigParams *params, SSLCertLookup *l
     
       // load the global ticket key for later use
       REC_ReadConfigStringAlloc(ticket_key_filename, "proxy.config.ssl.server.ticket_key.filename");
    -  ats_scoped_str ticket_key_path(Layout::relative_to(params->serverCertPathOnly, ticket_key_filename));
    -  global_default_keyblock = ssl_create_ticket_keyblock(ticket_key_path); // this function just returns a keyblock
    +  if (ticket_key_filename && strcmp(ticket_key_filename, "\"\"") != 0) {
    +    ats_scoped_str ticket_key_path(Layout::relative_to(params->serverCertPathOnly, ticket_key_filename));
    +    global_default_keyblock = ssl_create_ticket_keyblock(ticket_key_path); // this function just returns a keyblock
    +  } else {
    +    global_default_keyblock = ssl_create_ticket_keyblock(NULL);
    +  }
    --- End diff --
    
    That was a work around to get around the keyblock issue.  Removed it now that I rebased.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78410082
  
    --- Diff: lib/records/RecHttp.cc ---
    @@ -618,6 +633,30 @@ ts_session_protocol_well_known_name_indices_init()
       DEFAULT_TLS_SESSION_PROTOCOL_SET.markAllIn();
     
       DEFAULT_NON_TLS_SESSION_PROTOCOL_SET = HTTP_PROTOCOL_SET;
    +
    +  TSProtoTags = ink_hash_table_create(InkHashTableKeyType_String);
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_HTTP_1_0, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_HTTP_1_0)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_HTTP_1_1, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_HTTP_1_1)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_HTTP_2_0, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_HTTP_2_0)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TLS_1_3, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TLS_1_3)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TLS_1_2, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TLS_1_2)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TLS_1_1, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TLS_1_1)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TLS_1_0, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TLS_1_0)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TCP, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TCP)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_UDP, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_UDP)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_IPV4, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_IPV4)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_IPV6, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_IPV6)));
    --- End diff --
    
    At the point where you have to do ``reinterpret_cast<void *>(const_cast<char *>)``, you might as well give up and just use a C-style cast :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1007: TS-4703: Add API to retrieve client protocols

Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the issue:

    https://github.com/apache/trafficserver/pull/1007
  
    Ship it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78410287
  
    --- Diff: lib/records/RecHttp.cc ---
    @@ -618,6 +633,30 @@ ts_session_protocol_well_known_name_indices_init()
       DEFAULT_TLS_SESSION_PROTOCOL_SET.markAllIn();
     
       DEFAULT_NON_TLS_SESSION_PROTOCOL_SET = HTTP_PROTOCOL_SET;
    +
    +  TSProtoTags = ink_hash_table_create(InkHashTableKeyType_String);
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_HTTP_1_0, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_HTTP_1_0)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_HTTP_1_1, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_HTTP_1_1)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_HTTP_2_0, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_HTTP_2_0)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TLS_1_3, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TLS_1_3)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TLS_1_2, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TLS_1_2)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TLS_1_1, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TLS_1_1)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TLS_1_0, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TLS_1_0)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TCP, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TCP)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_UDP, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_UDP)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_IPV4, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_IPV4)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_IPV6, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_IPV6)));
    +}
    +
    +const char *
    +ts_normalize_proto_tag(const char *tag)
    --- End diff --
    
    I think APIs exported from librecords should be ``Rec*()``.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1007: TS-4703: Add API to retrieve client protocols

Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:

    https://github.com/apache/trafficserver/pull/1007
  
    [approve ci]


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78410221
  
    --- Diff: iocore/net/SSLUtils.cc ---
    @@ -2055,8 +2055,12 @@ SSLParseCertificateConfiguration(const SSLConfigParams *params, SSLCertLookup *l
     
       // load the global ticket key for later use
       REC_ReadConfigStringAlloc(ticket_key_filename, "proxy.config.ssl.server.ticket_key.filename");
    -  ats_scoped_str ticket_key_path(Layout::relative_to(params->serverCertPathOnly, ticket_key_filename));
    -  global_default_keyblock = ssl_create_ticket_keyblock(ticket_key_path); // this function just returns a keyblock
    +  if (ticket_key_filename && strcmp(ticket_key_filename, "\"\"") != 0) {
    --- End diff --
    
    Is this supposed to be in this PR?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78409374
  
    --- Diff: iocore/net/NetVConnection.cc ---
    @@ -43,3 +44,39 @@ NetVConnection::cancel_OOB()
     {
       return;
     }
    +
    +const char *
    +NetVCOptions::get_proto_string() const
    +{
    +  const char *retval;
    +  switch (ip_proto) {
    +  case USE_TCP:
    +    retval = TS_PROTO_TAG_TCP;
    +    break;
    +  case USE_UDP:
    +    retval = TS_PROTO_TAG_UDP;
    +    break;
    +  default:
    +    retval = NULL;
    +    break;
    +  }
    +  return retval;
    +}
    --- End diff --
    
    I think this would be more succinct:
    ```C
    switch (ip_proto) {
      case USE_TCP:
        return TS_PROTO_TAG_TCP;
     case USE_UDP:
      return TS_PROTO_TAG_UDP;
     default:
        return NULL;
     }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78400064
  
    --- Diff: iocore/net/P_UnixNetVConnection.h ---
    @@ -170,6 +170,38 @@ class UnixNetVConnection : public NetVConnection
       /////////////////////////////////////////////////////////////////
       UnixNetVConnection();
     
    +  int
    +  populate_protocol(char const **results, int n) const
    +  {
    +    int retval = 0;
    +    if (n > 0) {
    +      results[0] = options.get_proto_string();
    --- End diff --
    
    {{results[retval++] = options...}}?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1007: TS-4703: Add API to retrieve client protocols

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1007
  
    Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/678/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78399230
  
    --- Diff: example/protocol-stack/protocol-stack.cc ---
    @@ -0,0 +1,62 @@
    +/** @file
    +
    +  an example hello world plugin
    --- End diff --
    
    Update this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78400907
  
    --- Diff: iocore/net/P_UnixNetVConnection.h ---
    @@ -170,6 +170,38 @@ class UnixNetVConnection : public NetVConnection
       /////////////////////////////////////////////////////////////////
       UnixNetVConnection();
     
    +  int
    +  populate_protocol(char const **results, int n) const
    +  {
    +    int retval = 0;
    +    if (n > 0) {
    +      results[0] = options.get_proto_string();
    +      retval++;
    +      if (n > 1) {
    +        results[1] = options.get_family_string();
    +        retval++;
    +      }
    +    }
    +    return retval;
    +  }
    +
    +  const char *
    +  protocol_contains(const char *tag) const
    +  {
    +    const char *retval   = NULL;
    +    unsigned int tag_len = strlen(tag);
    +    const char *test_tag = options.get_proto_string();
    +    if (tag_len <= strlen(test_tag) && strncmp(tag, test_tag, tag_len) == 0) {
    --- End diff --
    
    Don't need to do the `strlen`, `strncmp` will stop (and fail) if the protocol tag has a null termination.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1007: TS-4703: Add API to retrieve client protocols

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:

    https://github.com/apache/trafficserver/pull/1007
  
    @ogoodman can you please review?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78409866
  
    --- Diff: iocore/net/SSLUtils.cc ---
    @@ -2055,8 +2055,12 @@ SSLParseCertificateConfiguration(const SSLConfigParams *params, SSLCertLookup *l
     
       // load the global ticket key for later use
       REC_ReadConfigStringAlloc(ticket_key_filename, "proxy.config.ssl.server.ticket_key.filename");
    -  ats_scoped_str ticket_key_path(Layout::relative_to(params->serverCertPathOnly, ticket_key_filename));
    -  global_default_keyblock = ssl_create_ticket_keyblock(ticket_key_path); // this function just returns a keyblock
    +  if (ticket_key_filename && strcmp(ticket_key_filename, "\"\"") != 0) {
    +    ats_scoped_str ticket_key_path(Layout::relative_to(params->serverCertPathOnly, ticket_key_filename));
    +    global_default_keyblock = ssl_create_ticket_keyblock(ticket_key_path); // this function just returns a keyblock
    +  } else {
    +    global_default_keyblock = ssl_create_ticket_keyblock(NULL);
    +  }
    --- End diff --
    
    This looks like it is from some other commit or branch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78399038
  
    --- Diff: doc/reference/api/TSClientProtocolStack.en.rst ---
    @@ -0,0 +1,79 @@
    +.. Licensed to the Apache Software Foundation (ASF) under one or more
    +   contributor license agreements.  See the NOTICE file distributed
    +   with this work for additional information regarding copyright
    +   ownership.  The ASF licenses this file to you under the Apache
    +   License, Version 2.0 (the "License"); you may not use this file
    +   except in compliance with the License.  You may obtain a copy of
    +   the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +   Unless required by applicable law or agreed to in writing, software
    +   distributed under the License is distributed on an "AS IS" BASIS,
    +   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +   implied.  See the License for the specific language governing
    +   permissions and limitations under the License.
    +
    +
    +.. include:: ../../../common.defs
    +
    +.. default-domain:: c
    +
    +TSClientProtocolStack
    +*********************
    +
    +Synopsis
    +========
    +
    +`#include <ts/ts.h>`
    +
    +.. function:: TSReturnCode TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const** result, int* actual)
    +
    +.. function:: TSReturnCode TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const** result, int* actual)
    +
    +.. function:: int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const** tag)
    +
    +.. function:: int TSHttpSsnClientProtocolStackContains(TSHttpSsn ssnp, char const** tag)
    +
    +.. function:: char const* TSNormalizedProtocolTag(char const* tag)
    +
    +.. function:: char const* TSRegisterProtocolTag(char const* tag)
    +
    +Description
    +===========
    +
    +These functions are used to explore the protocol stack of the client (user agent) connection to |TS|. The functions :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` can be used to retrieve the entire protocol stack for the user agent connection. :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` will check for a specific protocol :arg:`tag` being present in the stack.
    +
    +Each protocol is represented by tag which is a null terminated string. A particular tag will always be returned as the same character pointer and so protocols can be reliably checked with pointer comparisons. :func:`TSNormalizedProtocolTag` will return this character pointer for a specific :arg:`tag`. A return value of :const:`NULL` indicates the provided :arg:`tag` is not registered as a known protocol tag. :func:`TSRegisterProtocolTag` registers the :arg:`tag` and then returns its normalized value. This is useful for plugins that provide custom protocols for user agents.
    +
    +The protocols are ordered from higher level protocols to the lower level ones on which the higher operate. For instance a stack might look like "http/1.1,tls/1.2,tcp,ipv4". For :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` these values are placed in the array :arg:`result`. :arg:`count` is the maximum number of elements of :arg:`result` that may be modified by the function call. If :arg:`actual` is not :const:`NULL` then the actual number of elements in the protocol stack will be returned. If this is equal or less than :arg:`count` then all elements were returned. If it is larger then some layers were omitted from :arg:`result`. If the full stack is required :arg:`actual` can be used to resize :arg:`result` to be sufficient to hold all of the elements and the function called again with updated :arg:`count` and :arg:`result`. In practice the maximum number of elements will is almost certain to be less than 10 which therefore should suffice. T
 hese functions return :const:`TS_SUCCESS` on success and :const:`TS_ERROR` on failure which should only occurr if :arg:`txnp` or :arg:`ssnp` are invalid.
    +
    +The :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` functions are provided for the convenience when only the presence of a protocol is of interest, not its location or the presence of other protocols. These functions return 0 if the protocol :arg:`tag` is not present, non-zero if it is present. The strings are matched with an anchor prefix search, as with debug tags. For instance if :arg:`tag` is "tls" then it will match "tls/1.2" or "tls/1.3". This makes checking for TLS or IP more convenient. If more precision is required the entire protocol stack can be retrieved and processed more thoroughly.
    +
    +The protocol tags defined by |TS|.
    +
    +=========== =========
    +Protocol    Tag
    +=========== =========
    +HTTP/1.1    http/1.1
    +HTTP/1.0    http/1.0
    +HTTP/2      h2
    +WebSocket   ws
    +TLS 1.3     tls/1.3
    +TLS 1.2     tls/1.2
    --- End diff --
    
    Add TLS/1.1 and TLS/1.0


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by shinrich <gi...@git.apache.org>.
Github user shinrich closed the pull request at:

    https://github.com/apache/trafficserver/pull/1007


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78399173
  
    --- Diff: example/Makefile.am ---
    @@ -46,7 +47,7 @@ plugins = \
       ssl-sni.la \
       thread-1.la \
       txn-data-sink.la \
    -  version.la
    +  version.la 
    --- End diff --
    
    Remove trailing space.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78408588
  
    --- Diff: doc/reference/api/TSClientProtocolStack.en.rst ---
    @@ -0,0 +1,79 @@
    +.. Licensed to the Apache Software Foundation (ASF) under one or more
    +   contributor license agreements.  See the NOTICE file distributed
    +   with this work for additional information regarding copyright
    +   ownership.  The ASF licenses this file to you under the Apache
    +   License, Version 2.0 (the "License"); you may not use this file
    +   except in compliance with the License.  You may obtain a copy of
    +   the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +   Unless required by applicable law or agreed to in writing, software
    +   distributed under the License is distributed on an "AS IS" BASIS,
    +   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +   implied.  See the License for the specific language governing
    +   permissions and limitations under the License.
    +
    +
    +.. include:: ../../../common.defs
    +
    +.. default-domain:: c
    +
    +TSClientProtocolStack
    +*********************
    +
    +Synopsis
    +========
    +
    +`#include <ts/ts.h>`
    +
    +.. function:: TSReturnCode TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const** result, int* actual)
    +
    +.. function:: TSReturnCode TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const** result, int* actual)
    +
    +.. function:: int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const** tag)
    +
    +.. function:: int TSHttpSsnClientProtocolStackContains(TSHttpSsn ssnp, char const** tag)
    +
    +.. function:: char const* TSNormalizedProtocolTag(char const* tag)
    +
    +.. function:: char const* TSRegisterProtocolTag(char const* tag)
    +
    +Description
    +===========
    +
    +These functions are used to explore the protocol stack of the client (user agent) connection to |TS|. The functions :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` can be used to retrieve the entire protocol stack for the user agent connection. :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` will check for a specific protocol :arg:`tag` being present in the stack.
    +
    +Each protocol is represented by tag which is a null terminated string. A particular tag will always be returned as the same character pointer and so protocols can be reliably checked with pointer comparisons. :func:`TSNormalizedProtocolTag` will return this character pointer for a specific :arg:`tag`. A return value of :const:`NULL` indicates the provided :arg:`tag` is not registered as a known protocol tag. :func:`TSRegisterProtocolTag` registers the :arg:`tag` and then returns its normalized value. This is useful for plugins that provide custom protocols for user agents.
    +
    +The protocols are ordered from higher level protocols to the lower level ones on which the higher operate. For instance a stack might look like "http/1.1,tls/1.2,tcp,ipv4". For :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` these values are placed in the array :arg:`result`. :arg:`count` is the maximum number of elements of :arg:`result` that may be modified by the function call. If :arg:`actual` is not :const:`NULL` then the actual number of elements in the protocol stack will be returned. If this is equal or less than :arg:`count` then all elements were returned. If it is larger then some layers were omitted from :arg:`result`. If the full stack is required :arg:`actual` can be used to resize :arg:`result` to be sufficient to hold all of the elements and the function called again with updated :arg:`count` and :arg:`result`. In practice the maximum number of elements will is almost certain to be less than 10 which therefore should suffice. T
 hese functions return :const:`TS_SUCCESS` on success and :const:`TS_ERROR` on failure which should only occurr if :arg:`txnp` or :arg:`ssnp` are invalid.
    +
    +The :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` functions are provided for the convenience when only the presence of a protocol is of interest, not its location or the presence of other protocols. These functions return 0 if the protocol :arg:`tag` is not present, non-zero if it is present. The strings are matched with an anchor prefix search, as with debug tags. For instance if :arg:`tag` is "tls" then it will match "tls/1.2" or "tls/1.3". This makes checking for TLS or IP more convenient. If more precision is required the entire protocol stack can be retrieved and processed more thoroughly.
    --- End diff --
    
    Can you please line wrap this to something reasonable? 80 - 100?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1007: TS-4703: Add API to retrieve client protocols

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1007
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/782/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1007: TS-4703: Add API to retrieve client protocols

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1007
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/784/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78409027
  
    --- Diff: iocore/net/I_NetVConnection.h ---
    @@ -566,6 +570,17 @@ class NetVConnection : public VConnection
         is_transparent = state;
       }
     
    +  virtual int
    +  populate_protocol(char const **results, int n) const
    +  {
    +    return 0;
    +  }
    --- End diff --
    
    Line break please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78410540
  
    --- Diff: proxy/InkAPI.cc ---
    @@ -9193,3 +9193,71 @@ TSHttpTxnIdGet(TSHttpTxn txnp)
     
       return (uint64_t)sm->sm_id;
     }
    +
    +// Return information about the protocols used by the client
    +TSReturnCode
    +TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const **result, int *actual)
    +{
    +  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
    +  sdk_assert(result != NULL);
    +  HttpSM *sm = (HttpSM *)txnp;
    +  int count  = 0;
    +  if (sm) {
    +    count = sm->populate_client_protocol(result, n);
    +  }
    +  if (actual) {
    +    *actual = count;
    --- End diff --
    
    Is ``result`` allowed to be ``NULL``? We should ``sdk_assert`` that ``result`` and ``actual`` are both either present or absent together.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1007: TS-4703: Add API to retrieve client protocols

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1007
  
    Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/676/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78410788
  
    --- Diff: proxy/http/HttpSM.cc ---
    @@ -7940,3 +7940,50 @@ HttpSM::is_redirect_required()
       }
       return redirect_required;
     }
    +
    +// Fill in the client protocols used.  Return the number of entries returned
    +int
    +HttpSM::populate_client_protocol(const char **result, int n) const
    +{
    +  int retval = 0;
    +  if (n > 0) {
    +    const char *proto = HttpSM::find_proto_string(t_state.hdr_info.client_request.version_get());
    +    if (proto) {
    +      result[0] = proto;
    +      retval    = 1;
    +      if (n > 1 && ua_session) {
    --- End diff --
    
    I'd use `retval` instead of the hard coded `1` just to be consistent.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78398988
  
    --- Diff: doc/reference/api/TSClientProtocolStack.en.rst ---
    @@ -0,0 +1,79 @@
    +.. Licensed to the Apache Software Foundation (ASF) under one or more
    +   contributor license agreements.  See the NOTICE file distributed
    +   with this work for additional information regarding copyright
    +   ownership.  The ASF licenses this file to you under the Apache
    +   License, Version 2.0 (the "License"); you may not use this file
    +   except in compliance with the License.  You may obtain a copy of
    +   the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +   Unless required by applicable law or agreed to in writing, software
    +   distributed under the License is distributed on an "AS IS" BASIS,
    +   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +   implied.  See the License for the specific language governing
    +   permissions and limitations under the License.
    +
    +
    +.. include:: ../../../common.defs
    +
    +.. default-domain:: c
    +
    +TSClientProtocolStack
    +*********************
    +
    +Synopsis
    +========
    +
    +`#include <ts/ts.h>`
    +
    +.. function:: TSReturnCode TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const** result, int* actual)
    +
    +.. function:: TSReturnCode TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const** result, int* actual)
    +
    +.. function:: int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const** tag)
    +
    +.. function:: int TSHttpSsnClientProtocolStackContains(TSHttpSsn ssnp, char const** tag)
    +
    +.. function:: char const* TSNormalizedProtocolTag(char const* tag)
    +
    +.. function:: char const* TSRegisterProtocolTag(char const* tag)
    +
    +Description
    +===========
    +
    +These functions are used to explore the protocol stack of the client (user agent) connection to |TS|. The functions :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` can be used to retrieve the entire protocol stack for the user agent connection. :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` will check for a specific protocol :arg:`tag` being present in the stack.
    +
    +Each protocol is represented by tag which is a null terminated string. A particular tag will always be returned as the same character pointer and so protocols can be reliably checked with pointer comparisons. :func:`TSNormalizedProtocolTag` will return this character pointer for a specific :arg:`tag`. A return value of :const:`NULL` indicates the provided :arg:`tag` is not registered as a known protocol tag. :func:`TSRegisterProtocolTag` registers the :arg:`tag` and then returns its normalized value. This is useful for plugins that provide custom protocols for user agents.
    +
    +The protocols are ordered from higher level protocols to the lower level ones on which the higher operate. For instance a stack might look like "http/1.1,tls/1.2,tcp,ipv4". For :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` these values are placed in the array :arg:`result`. :arg:`count` is the maximum number of elements of :arg:`result` that may be modified by the function call. If :arg:`actual` is not :const:`NULL` then the actual number of elements in the protocol stack will be returned. If this is equal or less than :arg:`count` then all elements were returned. If it is larger then some layers were omitted from :arg:`result`. If the full stack is required :arg:`actual` can be used to resize :arg:`result` to be sufficient to hold all of the elements and the function called again with updated :arg:`count` and :arg:`result`. In practice the maximum number of elements will is almost certain to be less than 10 which therefore should suffice. T
 hese functions return :const:`TS_SUCCESS` on success and :const:`TS_ERROR` on failure which should only occurr if :arg:`txnp` or :arg:`ssnp` are invalid.
    +
    +The :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` functions are provided for the convenience when only the presence of a protocol is of interest, not its location or the presence of other protocols. These functions return 0 if the protocol :arg:`tag` is not present, non-zero if it is present. The strings are matched with an anchor prefix search, as with debug tags. For instance if :arg:`tag` is "tls" then it will match "tls/1.2" or "tls/1.3". This makes checking for TLS or IP more convenient. If more precision is required the entire protocol stack can be retrieved and processed more thoroughly.
    +
    +The protocol tags defined by |TS|.
    +
    +=========== =========
    +Protocol    Tag
    +=========== =========
    +HTTP/1.1    http/1.1
    +HTTP/1.0    http/1.0
    +HTTP/2      h2
    +WebSocket   ws
    +TLS 1.3     tls/1.3
    +TLS 1.2     tls/1.2
    +TCP         tcp
    +UDP         udp
    +IPv4        ipv4
    +IPv6        ipv6
    +QUIC        quic
    +=========== =========
    +
    +.. note::
    --- End diff --
    
    We decided on this, the note should be junked.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78410759
  
    --- Diff: proxy/InkAPI.cc ---
    @@ -9193,3 +9193,71 @@ TSHttpTxnIdGet(TSHttpTxn txnp)
     
       return (uint64_t)sm->sm_id;
     }
    +
    +// Return information about the protocols used by the client
    +TSReturnCode
    +TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const **result, int *actual)
    +{
    +  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
    +  sdk_assert(result != NULL);
    +  HttpSM *sm = (HttpSM *)txnp;
    +  int count  = 0;
    +  if (sm) {
    +    count = sm->populate_client_protocol(result, n);
    +  }
    +  if (actual) {
    +    *actual = count;
    +  }
    +  return TS_SUCCESS;
    +}
    +
    +TSReturnCode
    +TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const **result, int *actual)
    +{
    +  sdk_assert(sdk_sanity_check_http_ssn(ssnp) == TS_SUCCESS);
    +  ProxyClientSession *cs = reinterpret_cast<ProxyClientSession *>(ssnp);
    +  int count              = 0;
    +  if (cs) {
    +    count = cs->populate_protocol(result, n);
    +  }
    +  if (actual) {
    +    *actual = count;
    +  }
    +  return TS_SUCCESS;
    +}
    +
    +char const *
    +TSNormalizedProtocolTag(char const *tag)
    +{
    +  return ts_normalize_proto_tag(tag);
    +}
    +
    +int
    +TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *tag, char const **ret_tag)
    --- End diff --
    
    This is not the function signature you documented.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1007: TS-4703: Add API to retrieve client protocols

Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:

    https://github.com/apache/trafficserver/pull/1007
  
    Don't know why the linux build failed.  Complained about a signed mismatch warning/error in a file that hasn't changed.
    
     CC       TsConfigSyntax.lo
    TsConfigSyntax.c: In function 'tsconfig_scan_bytes':
    TsConfigSyntax.c:1714:17: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
      for ( i = 0; i < _yybytes_len; ++i )
                     ^


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78435282
  
    --- Diff: proxy/InkAPI.cc ---
    @@ -9193,3 +9193,71 @@ TSHttpTxnIdGet(TSHttpTxn txnp)
     
       return (uint64_t)sm->sm_id;
     }
    +
    +// Return information about the protocols used by the client
    +TSReturnCode
    +TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const **result, int *actual)
    +{
    +  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
    +  sdk_assert(result != NULL);
    +  HttpSM *sm = (HttpSM *)txnp;
    +  int count  = 0;
    +  if (sm) {
    +    count = sm->populate_client_protocol(result, n);
    +  }
    +  if (actual) {
    +    *actual = count;
    +  }
    +  return TS_SUCCESS;
    +}
    +
    +TSReturnCode
    +TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const **result, int *actual)
    +{
    +  sdk_assert(sdk_sanity_check_http_ssn(ssnp) == TS_SUCCESS);
    +  ProxyClientSession *cs = reinterpret_cast<ProxyClientSession *>(ssnp);
    +  int count              = 0;
    +  if (cs) {
    +    count = cs->populate_protocol(result, n);
    +  }
    +  if (actual) {
    +    *actual = count;
    +  }
    +  return TS_SUCCESS;
    +}
    +
    +char const *
    +TSNormalizedProtocolTag(char const *tag)
    +{
    +  return ts_normalize_proto_tag(tag);
    +}
    +
    +int
    +TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *tag, char const **ret_tag)
    --- End diff --
    
    Agreed.  Making the change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78410969
  
    --- Diff: proxy/InkAPI.cc ---
    @@ -9193,3 +9193,71 @@ TSHttpTxnIdGet(TSHttpTxn txnp)
     
       return (uint64_t)sm->sm_id;
     }
    +
    +// Return information about the protocols used by the client
    +TSReturnCode
    +TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const **result, int *actual)
    +{
    +  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
    +  sdk_assert(result != NULL);
    +  HttpSM *sm = (HttpSM *)txnp;
    +  int count  = 0;
    +  if (sm) {
    +    count = sm->populate_client_protocol(result, n);
    +  }
    +  if (actual) {
    +    *actual = count;
    +  }
    +  return TS_SUCCESS;
    +}
    +
    +TSReturnCode
    +TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const **result, int *actual)
    +{
    +  sdk_assert(sdk_sanity_check_http_ssn(ssnp) == TS_SUCCESS);
    +  ProxyClientSession *cs = reinterpret_cast<ProxyClientSession *>(ssnp);
    +  int count              = 0;
    +  if (cs) {
    +    count = cs->populate_protocol(result, n);
    +  }
    +  if (actual) {
    +    *actual = count;
    +  }
    +  return TS_SUCCESS;
    +}
    +
    +char const *
    +TSNormalizedProtocolTag(char const *tag)
    +{
    +  return ts_normalize_proto_tag(tag);
    +}
    +
    +int
    +TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *tag, char const **ret_tag)
    --- End diff --
    
    This doesn't need an output parameter. We could do this:
    ```C
    const char *
    TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *tag);
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78411573
  
    --- Diff: proxy/http2/Http2ClientSession.h ---
    @@ -243,6 +243,32 @@ class Http2ClientSession : public ProxyClientSession
         return "http/2";
       }
     
    +  virtual int
    +  populate_protocol(char const **result, int size) const
    +  {
    +    int retval = 0;
    +    if (size > 0) {
    +      result[0] = TS_PROTO_TAG_HTTP_2_0;
    +      retval    = 1;
    +      if (size > 1) {
    +        retval += super::populate_protocol(result + 1, size - 1);
    +      }
    +    }
    +    return retval;
    +  }
    --- End diff --
    
    Could you add a newline here (and in other places between functions) please?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1007: TS-4703: Add API to retrieve client protocols

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:

    https://github.com/apache/trafficserver/pull/1007
  
    ```
    Making all in example
    ...
    make[1]: *** No rule to make target `protocol-stack/protocol-stack.c', needed by `protocol-stack/protocol-stack.lo'.  Stop.
    make[1]: *** Waiting for unfinished jobs....
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78410472
  
    --- Diff: proxy/InkAPI.cc ---
    @@ -9193,3 +9193,71 @@ TSHttpTxnIdGet(TSHttpTxn txnp)
     
       return (uint64_t)sm->sm_id;
     }
    +
    +// Return information about the protocols used by the client
    +TSReturnCode
    +TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const **result, int *actual)
    +{
    +  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
    +  sdk_assert(result != NULL);
    --- End diff --
    
    `assert(n == 0 || result != NULL);`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78403317
  
    --- Diff: proxy/ProxyClientSession.h ---
    @@ -184,6 +184,25 @@ class ProxyClientSession : public VConnection
         return get_netvc() == NULL;
       }
     
    +  virtual int
    +  populate_protocol(char const **result, int size) const
    +  {
    +    int retval = 0;
    +    if (get_netvc()) {
    +      retval += this->get_netvc()->populate_protocol(result, size);
    +    }
    +    return retval;
    +  }
    --- End diff --
    
    Newline here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78411137
  
    --- Diff: proxy/api/ts/ts.h ---
    @@ -2419,6 +2419,16 @@ tsapi const TSUuid TSProcessUuidGet(void);
     */
     tsapi const char *TSHttpTxnPluginTagGet(TSHttpTxn txnp);
     
    +/*
    + * Return information about the client protocols
    + */
    +tsapi TSReturnCode TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const **result, int *actual);
    +tsapi TSReturnCode TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const **result, int *actual);
    +tsapi int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *tag, char const **ret_tag);
    +tsapi int TSHttpSsnClientProtocolStackContains(TSHttpSsn ssnp, char const *tag, char const **ret_tag);
    +tsapi char const *TSNormalizedProtocolTag(char const *tag);
    +tsapi char const *TSRegisterProtocolTag(char const *tag);
    --- End diff --
    
    Can we please be consistent about ``const char`` and ``char const``? Let's choose one or the other and file a separate Jira to make the API consistent.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78436051
  
    --- Diff: proxy/api/ts/ts.h ---
    @@ -2419,6 +2419,16 @@ tsapi const TSUuid TSProcessUuidGet(void);
     */
     tsapi const char *TSHttpTxnPluginTagGet(TSHttpTxn txnp);
     
    +/*
    + * Return information about the client protocols
    + */
    +tsapi TSReturnCode TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const **result, int *actual);
    +tsapi TSReturnCode TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const **result, int *actual);
    +tsapi int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *tag, char const **ret_tag);
    +tsapi int TSHttpSsnClientProtocolStackContains(TSHttpSsn ssnp, char const *tag, char const **ret_tag);
    +tsapi char const *TSNormalizedProtocolTag(char const *tag);
    +tsapi char const *TSRegisterProtocolTag(char const *tag);
    --- End diff --
    
    Will file a bug to address the char const vs const char issue.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1007: TS-4703: Add API to retrieve client protocols

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1007
  
    Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/680/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78408638
  
    --- Diff: example/Makefile.am ---
    @@ -68,6 +69,7 @@ lifecycle_plugin_la_SOURCES = lifecycle-plugin/lifecycle-plugin.c
     null_transform_la_SOURCES = null-transform/null-transform.c
     output_header_la_SOURCES = output-header/output-header.c
     protocol_la_SOURCES = protocol/Protocol.c protocol/TxnSM.c
    +protocol_stack_la_SOURCES = protocol-stack/protocol-stack.c
    --- End diff --
    
    ``protocol-stack.cc``


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78434727
  
    --- Diff: iocore/net/SSLUtils.cc ---
    @@ -2055,8 +2055,12 @@ SSLParseCertificateConfiguration(const SSLConfigParams *params, SSLCertLookup *l
     
       // load the global ticket key for later use
       REC_ReadConfigStringAlloc(ticket_key_filename, "proxy.config.ssl.server.ticket_key.filename");
    -  ats_scoped_str ticket_key_path(Layout::relative_to(params->serverCertPathOnly, ticket_key_filename));
    -  global_default_keyblock = ssl_create_ticket_keyblock(ticket_key_path); // this function just returns a keyblock
    +  if (ticket_key_filename && strcmp(ticket_key_filename, "\"\"") != 0) {
    --- End diff --
    
    Nope as noted with James' comment.  Was a workaround for the keyblock problem.  Gone with the rebase.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1007: TS-4703: Add API to retrieve client protocols

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1007
  
    FreeBSD build *failed*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/780/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1007: TS-4703: Add API to retrieve client protocols

Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:

    https://github.com/apache/trafficserver/pull/1007
  
    added TS-4855 to track the "const char" vs "char const" issue.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78408986
  
    --- Diff: example/protocol-stack/protocol-stack.cc ---
    @@ -0,0 +1,62 @@
    +/** @file
    +
    +  an example hello world plugin
    +
    +  @section license License
    +
    +  Licensed to the Apache Software Foundation (ASF) under one
    +  or more contributor license agreements.  See the NOTICE file
    +  distributed with this work for additional information
    +  regarding copyright ownership.  The ASF licenses this file
    +  to you under the Apache License, Version 2.0 (the
    +  "License"); you may not use this file except in compliance
    +  with the License.  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    + */
    +
    +#include <stdio.h>
    +
    +#include "ts/ts.h"
    +#include "ts/ink_defs.h"
    +
    +#define DEBUG_TAG "protocol-stack"
    +
    +static int
    +proto_stack_cb(TSCont contp ATS_UNUSED, TSEvent event, void *edata)
    +{
    +  TSHttpTxn txnp = (TSHttpTxn)edata;
    +  char const *results[10];
    +  int count = 0;
    +  TSDebug(DEBUG_TAG, "Protocols:");
    +  TSHttpTxnClientProtocolStackGet(txnp, 10, results, &count);
    +  for (int i = 0; i < count; i++) {
    +    TSDebug(DEBUG_TAG, "\t%d: %s", i, results[i]);
    +  }
    +  int ret = TSHttpTxnClientProtocolStackContains(txnp, "h2", NULL);
    +  TSDebug(DEBUG_TAG, "Stack %s HTTP/2", ret == 1 ? "contains" : "does not contain");
    +  TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
    --- End diff --
    
    Use ``literalinclude`` to include this code sample in the man page (see other examples of this in the docs tree).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78416925
  
    --- Diff: proxy/InkAPI.cc ---
    @@ -9193,3 +9193,71 @@ TSHttpTxnIdGet(TSHttpTxn txnp)
     
       return (uint64_t)sm->sm_id;
     }
    +
    +// Return information about the protocols used by the client
    +TSReturnCode
    +TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const **result, int *actual)
    +{
    +  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
    +  sdk_assert(result != NULL);
    +  HttpSM *sm = (HttpSM *)txnp;
    +  int count  = 0;
    +  if (sm) {
    +    count = sm->populate_client_protocol(result, n);
    +  }
    +  if (actual) {
    +    *actual = count;
    --- End diff --
    
    My view, as I noted elsewhere in comments, is `result` can be `NULL` if `n` is `0`. Kind of a dumb thing to pass, but it could happen due to the caller making computations.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1007: TS-4703: Add API to retrieve client protoc...

Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1007#discussion_r78434544
  
    --- Diff: lib/records/RecHttp.cc ---
    @@ -618,6 +633,30 @@ ts_session_protocol_well_known_name_indices_init()
       DEFAULT_TLS_SESSION_PROTOCOL_SET.markAllIn();
     
       DEFAULT_NON_TLS_SESSION_PROTOCOL_SET = HTTP_PROTOCOL_SET;
    +
    +  TSProtoTags = ink_hash_table_create(InkHashTableKeyType_String);
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_HTTP_1_0, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_HTTP_1_0)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_HTTP_1_1, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_HTTP_1_1)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_HTTP_2_0, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_HTTP_2_0)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TLS_1_3, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TLS_1_3)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TLS_1_2, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TLS_1_2)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TLS_1_1, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TLS_1_1)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TLS_1_0, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TLS_1_0)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_TCP, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_TCP)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_UDP, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_UDP)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_IPV4, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_IPV4)));
    +  ink_hash_table_insert(TSProtoTags, TS_PROTO_TAG_IPV6, reinterpret_cast<void *>(const_cast<char *>(TS_PROTO_TAG_IPV6)));
    --- End diff --
    
    But then Alan will yell at me...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---