You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by SolidWallOfCode <gi...@git.apache.org> on 2016/11/01 14:48:45 UTC

[GitHub] trafficserver pull request #1167: TS-5025: Add 'setHost' method to 'atscppap...

GitHub user SolidWallOfCode opened a pull request:

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

    TS-5025: Add 'setHost' method to 'atscppapi::Request'

    

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

    $ git pull https://github.com/SolidWallOfCode/trafficserver ts-5025

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

    https://github.com/apache/trafficserver/pull/1167.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 #1167
    
----
commit 250f49d64b0cb222c448c8d48628bdb3c757211b
Author: Alan M. Carroll <so...@yahoo-inc.com>
Date:   2016-11-01T14:29:00Z

    TS-5025: Add "setHost" method to atscppapi::Request

----


---
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 #1167: TS-5025: Add 'setHost' method to 'atscppap...

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

    https://github.com/apache/trafficserver/pull/1167#discussion_r85967946
  
    --- Diff: lib/cppapi/Request.cc ---
    @@ -177,6 +177,23 @@ Request::getHeaders() const
       return state_->headers_;
     }
     
    +void
    +Request::setHost(std::string const &host)
    +{
    +  static std::string HOST_FIELD_NAME(TS_MIME_FIELD_HOST, TS_MIME_LEN_HOST);
    --- End diff --
    
    < bike_shed >Shouldn't this be const too?< /bike_shed >


---
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 #1167: TS-5025: Add 'setHost' method to 'atscppap...

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

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


---
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 #1167: TS-5025: Add 'setHost' method to 'atscppapi::Requ...

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

    https://github.com/apache/trafficserver/pull/1167
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1134/ 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 #1167: TS-5025: Add 'setHost' method to 'atscppap...

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

    https://github.com/apache/trafficserver/pull/1167#discussion_r86015275
  
    --- Diff: lib/cppapi/Request.cc ---
    @@ -177,6 +177,23 @@ Request::getHeaders() const
       return state_->headers_;
     }
     
    +void
    +Request::setHost(std::string const &host)
    +{
    +  static std::string HOST_FIELD_NAME(TS_MIME_FIELD_HOST, TS_MIME_LEN_HOST);
    --- End diff --
    
    Yes it should.


---
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 #1167: TS-5025: Add 'setHost' method to 'atscppapi::Requ...

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

    https://github.com/apache/trafficserver/pull/1167
  
    Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/1028/ 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 #1167: TS-5025: Add 'setHost' method to 'atscppapi::Requ...

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

    https://github.com/apache/trafficserver/pull/1167
  
    Yeah, we had a problem at Y! this fixed. The plugin writer set the `HOST` header in order to control subsequent remapping, a reasonable thing to do. But if the request comes in with the host in the URL then remap prioritizes that over the `HOST` header and things don't work right for non obvious reasons. Better to have a real "set the host" method that does the right thing. Technically this could be done by the plugin but the point of the CPP API is to make writing plugins easier by taking care of these pesky details that would be the same in every plugin.


---
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.
---