You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2022/06/08 07:43:17 UTC

[GitHub] [trafficserver] masaori335 opened a new pull request, #8893: Fix % with PROXY Protocol

masaori335 opened a new pull request, #8893:
URL: https://github.com/apache/trafficserver/pull/8893

   Fix #8544.
   
   Prior to this change, the source address in the PROXY Protocol message was set as the remote address when it was parsed.
   Now, `%<chi>` always represents the previous hop.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] zwoop commented on pull request #8893: Fix % with PROXY Protocol

Posted by GitBox <gi...@apache.org>.
zwoop commented on PR #8893:
URL: https://github.com/apache/trafficserver/pull/8893#issuecomment-1177891378

   It kinda feels like this is borderline incompatible change, in that the logging behavior of %{chi} could possibly change. The odds of this happening is small, but we should consider this and make sure we don't break the compatibility contracts.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] masaori335 commented on a diff in pull request #8893: Fix % with PROXY Protocol

Posted by GitBox <gi...@apache.org>.
masaori335 commented on code in PR #8893:
URL: https://github.com/apache/trafficserver/pull/8893#discussion_r892030439


##########
iocore/net/SSLNetVConnection.cc:
##########
@@ -455,7 +454,6 @@ SSLNetVConnection::read_raw_data()
 
     if (this->has_proxy_protocol(buffer, &r)) {
       Debug("proxyprotocol", "ssl has proxy protocol header");
-      set_remote_addr(get_proxy_protocol_src_addr());

Review Comment:
   I'm wondering if we can unify these code in the `SSLNetVConnection` and `ProtocolProbeSessionAccept`, but it's out of scope from this PR.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] masaori335 merged pull request #8893: Fix % with PROXY Protocol

Posted by GitBox <gi...@apache.org>.
masaori335 merged PR #8893:
URL: https://github.com/apache/trafficserver/pull/8893


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] masaori335 commented on a diff in pull request #8893: Fix % with PROXY Protocol

Posted by GitBox <gi...@apache.org>.
masaori335 commented on code in PR #8893:
URL: https://github.com/apache/trafficserver/pull/8893#discussion_r892026814


##########
proxy/ProtocolProbeSessionAccept.cc:
##########
@@ -123,7 +123,6 @@ struct ProtocolProbeTrampoline : public Continuation, public ProtocolProbeSessio
 
       if (netvc->has_proxy_protocol(reader)) {
         Debug("proxyprotocol", "ioCompletionEvent: http has proxy protocol header");
-        netvc->set_remote_addr(netvc->get_proxy_protocol_src_addr());

Review Comment:
   This is the main change.



##########
iocore/net/SSLNetVConnection.cc:
##########
@@ -455,7 +454,6 @@ SSLNetVConnection::read_raw_data()
 
     if (this->has_proxy_protocol(buffer, &r)) {
       Debug("proxyprotocol", "ssl has proxy protocol header");
-      set_remote_addr(get_proxy_protocol_src_addr());

Review Comment:
   This is the main change.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] masaori335 commented on pull request #8893: Fix % with PROXY Protocol

Posted by GitBox <gi...@apache.org>.
masaori335 commented on PR #8893:
URL: https://github.com/apache/trafficserver/pull/8893#issuecomment-1154560921

   The doc of `%<chi>` is updated. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org