Fixed "Cannot assign requested address" for to-origin TPROXY FTP data#142
Fixed "Cannot assign requested address" for to-origin TPROXY FTP data#142rousskov merged 1 commit intosquid-cache:masterfrom
Conversation
|
Can one of the admins verify this patch? |
|
OK to test |
rousskov
left a comment
rousskov
left a comment
There was a problem hiding this comment.
See inlined comments for one bug and a question.
src/clients/FtpClient.cc
Outdated
| conn->tos = ctrl.conn->tos; | ||
| conn->nfmark = ctrl.conn->nfmark; | ||
| // Using non-local addresses in TPROXY mode requires appropriate socket option. | ||
| conn->flags = ctrl.conn->flags & COMM_TRANSPARENT; |
There was a problem hiding this comment.
We should not be overwriting the entire conn->flags value like this. If you want to copy the COMM_TRANSPARENT bit from the control connection flags, please copy it without touching the other bits of the conn->flags.
Please note that the above would remain true even if conn->flags were always zero at the time of assignment in the current code (but it is not zero AFAICT).
src/clients/FtpClient.cc
Outdated
| conn->tos = ctrl.conn->tos; | ||
| conn->nfmark = ctrl.conn->nfmark; | ||
| // Using non-local addresses in TPROXY mode requires appropriate socket option. | ||
| conn->flags = ctrl.conn->flags & COMM_TRANSPARENT; |
There was a problem hiding this comment.
getOutgoingAddress(), which works with somewhat related concepts, also sets COMM_DOBIND. Do we need that flag here as well?
There was a problem hiding this comment.
On our setup everything works without COMM_DOBIND flag.
There was a problem hiding this comment.
DOBIND is only necessary if the IP address might be the magic ANY_ADDR. Which this FtpClient code is never permitting to happen (the ctrl->local should never be ANY_ADDR).
rousskov
changed the title
|
I updated PR title and description to use a more common error message text and to indicate that the bug affects Squid-to-origin connections. |
yadij
left a comment
yadij
left a comment
There was a problem hiding this comment.
Latest revision looks okay to me, assuming it works.
4 participants
No description provided.