Traffic Server
  1. Traffic Server
  2. TS-1127

Wrong returned value of incoming port address

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.1.2
    • Fix Version/s: 3.1.4
    • Component/s: HTTP
    • Labels:
      None

      Description

      The API TSHttpTxnClientIncomingPortGet has been changed in Wed Oct 5 19:14:07 2011 (TS-926) and it returns another value.
      in the old version it returned the incoming port of the TS(the port which the client connected to the TS).
      in the new version the returned value is the sending port of the user.
      The different is in the line:

      • return sm->t_state.client_info.port;
        + return ink_inet_get_port(&sm->t_state.client_info.addr);

      The assignment of those two members (port, addr) are in the HttpSM.cc file

      ink_inet_copy(&t_state.client_info.addr, netvc->get_remote_addr());
      t_state.client_info.port = netvc->get_local_port();

      The old code gave the right answer from the port member, and the new one gives us wrong answer from the remote address.

      I attached a patch to fix this returned value.

      1. fix.patch
        0.4 kB
        Yakov Kopel

        Activity

        Leif Hedstrom made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Leif Hedstrom made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Leif Hedstrom added a comment -

        Fixed in

        commit ceb6467405db5c6923bb7e46a75d156e33ea1c60
        Author: Leif Hedstrom <leif@ogre.com>
        Commit: Leif Hedstrom <leif@ogre.com>
        
            TS-1127 Also fix the regression tests
        
        commit 03d6ea1748aa2878c6e8d5c91b1ac0258e60c30c
        Author: Yakov Kopel <ykopel@websense.com>
        Commit: Leif Hedstrom <leif@ogre.com>
        
            TS-1127 Wrong returned value of incoming port address.
            Author: Yakov Kopel
        
        
        Show
        Leif Hedstrom added a comment - Fixed in commit ceb6467405db5c6923bb7e46a75d156e33ea1c60 Author: Leif Hedstrom <leif@ogre.com> Commit: Leif Hedstrom <leif@ogre.com> TS-1127 Also fix the regression tests commit 03d6ea1748aa2878c6e8d5c91b1ac0258e60c30c Author: Yakov Kopel <ykopel@websense.com> Commit: Leif Hedstrom <leif@ogre.com> TS-1127 Wrong returned value of incoming port address. Author: Yakov Kopel
        Hide
        Leif Hedstrom added a comment -

        Fwiw, this is a deprecated API (which is why the regressions missed it).

        Show
        Leif Hedstrom added a comment - Fwiw, this is a deprecated API (which is why the regressions missed it).
        Hide
        Leif Hedstrom added a comment -

        Taking this, reading the docs, and looking at the code, I'm 99% certain the proposed patch is correct.

        Show
        Leif Hedstrom added a comment - Taking this, reading the docs, and looking at the code, I'm 99% certain the proposed patch is correct.
        Leif Hedstrom made changes -
        Assignee Alan M. Carroll [ amc ] Leif Hedstrom [ zwoop ]
        Leif Hedstrom made changes -
        Fix Version/s 3.1.4 [ 12318543 ]
        Fix Version/s 3.1.3 [ 12317969 ]
        Leif Hedstrom made changes -
        Assignee Alan M. Carroll [ amc ]
        Leif Hedstrom made changes -
        Fix Version/s 3.1.3 [ 12317969 ]
        Fix Version/s 3.1.4 [ 12318543 ]
        Hide
        Leif Hedstrom added a comment -

        amc, can you take a look at this?

        Show
        Leif Hedstrom added a comment - amc, can you take a look at this?
        Yakov Kopel made changes -
        Attachment fix.patch [ 12517067 ]
        Yakov Kopel made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Yakov Kopel made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Yakov Kopel made changes -
        Summary Wrong assignment of incoming address Wrong returned value of incoming port address
        Yakov Kopel made changes -
        Attachment fix.patch [ 12517220 ]
        Yakov Kopel made changes -
        Description There is a problem in the new version of trafficserverin the function (HttpSM.cc):
        TSHttpTxnClientIncomingPortGet

        The old code:
        return sm->t_state.client_info.port
        The new code:
        return ink_inet_get_port(&sm->t_state.client_info.addr);

        The assignment of those two members (port, addr) are in the HttpSM.cc file
        (in attach_client_session function):

          ink_inet_copy(&t_state.client_info.addr, netvc->get_remote_addr());
          t_state.client_info.port = netvc->get_local_port();
          
        the old code gave the right answer from the port member, and the new one gives us wrong answer fron the remote addr.
        I thought to fix it by rewrite the code get_remote_addr => get_local_addr
        (as in the patch that I attached to here)
        But it makes bugs in other places (redirect to another address).

        Or maybe it will be better to rewrite the TSHttpTxnClientIncomingPortGet function to the old version.

        The API TSHttpTxnClientIncomingPortGet has been changed in Wed Oct 5 19:14:07 2011 (TS-926) and it returns another value.
        in the old version it returned the incoming port of the TS(the port which the client connected to the TS).
        in the new version the returned value is the sending port of the user.
        The different is in the line:
          - return sm->t_state.client_info.port;
          + return ink_inet_get_port(&sm->t_state.client_info.addr);

        The assignment of those two members (port, addr) are in the HttpSM.cc file

          ink_inet_copy(&t_state.client_info.addr, netvc->get_remote_addr());
          t_state.client_info.port = netvc->get_local_port();
          
        The old code gave the right answer from the port member, and the new one gives us wrong answer from the remote address.

        I attached a patch to fix this returned value.
        Yakov Kopel made changes -
        Description In HttpSM.cc file (attach_client_session) there are two lines:
          ink_inet_copy(&t_state.client_info.addr, netvc->get_remote_addr());
          t_state.client_info.port = netvc->get_local_port();

        instead of:
          ink_inet_copy(&t_state.client_info.addr, netvc->get_local_addr());
          t_state.client_info.port = netvc->get_local_port();

        There is a problem in the new version of trafficserverin the function (HttpSM.cc):
        TSHttpTxnClientIncomingPortGet

        The old code:
        return sm->t_state.client_info.port
        The new code:
        return ink_inet_get_port(&sm->t_state.client_info.addr);

        The assignment of those two members (port, addr) are in the HttpSM.cc file
        (in attach_client_session function):

          ink_inet_copy(&t_state.client_info.addr, netvc->get_remote_addr());
          t_state.client_info.port = netvc->get_local_port();
          
        the old code gave the right answer from the port member, and the new one gives us wrong answer fron the remote addr.
        I thought to fix it by rewrite the code get_remote_addr => get_local_addr
        (as in the patch that I attached to here)
        But it makes bugs in other places (redirect to another address).

        Or maybe it will be better to rewrite the TSHttpTxnClientIncomingPortGet function to the old version.

        Yakov Kopel made changes -
        Attachment fix.patch [ 12517067 ]
        Yakov Kopel made changes -
        Field Original Value New Value
        Status Open [ 1 ] Patch Available [ 10002 ]
        Yakov Kopel created issue -

          People

          • Assignee:
            Leif Hedstrom
            Reporter:
            Yakov Kopel
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 1m
              1m
              Remaining:
              Remaining Estimate - 1m
              1m
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development