Github user JamesPeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/773#discussion_r71447768
--- Diff: proxy/http/HttpTransact.cc ---
@@ -1783,7 +1783,13 @@ HttpTransact::OSDNSLookup(State *s)
// update some state variables with hostdb information that has
// been provided.
ats_ip_copy(&s->server_info.dst_addr, s->host_db_info.ip());
- s->server_info.dst_addr.port() =
htons(s->hdr_info.client_request.port_get()); // now we can set the port.
+ // TODO ideally only if ports aren't defined in remap
+ // If the SRV response has a port number, we should honor it. Otherwise
we do the port defined in remap
+ if (s->dns_info.srv_port) {
+ s->server_info.dst_addr.port() = htons(s->dns_info.srv_port);
+ } else {
+ s->server_info.dst_addr.port() =
htons(s->hdr_info.client_request.port_get()); // now we can set the port.
+ }
--- End diff --
If a plugin used ``TSHttpTxnServerAddrSet()``, then at this point
``dst_addr`` may be the address set by the plugin in which case we should not
clobber it. However it is already being clobbered from the request URL, so
maybe the cases I tested were already broken (I was using port 80 and 443).
Setting the port from the SRV record should probably be conditional on
``srv_lookup_success``.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
