You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/12/06 11:06:27 UTC

[GitHub] [commons-net] SteveOswald opened a new pull request, #127: Fixing proxy-problems in _parsePassiveModeReply

SteveOswald opened a new pull request, #127:
URL: https://github.com/apache/commons-net/pull/127

   FTPClient in passive mode didn't worked, when using a proxy, cause the FTPClient tried to open a data-connection to the proxy-host instead the real passive-host. This was caused, cause the passiveHost was ALWAYS overwritten by the socket-address (wich is the address of the proxy-host, when using a proxy). The passive-host should ALWAYS be parsed from the pasvResponse and only be overwritten, when the pasvResponse is a "look lie IP address" (0,0,0,0).


-- 
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: issues-unsubscribe@commons.apache.org

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

[GitHub] [commons-net] garydgregory commented on pull request #127: Fixing proxy-problems in _parsePassiveModeReply

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #127:
URL: https://github.com/apache/commons-net/pull/127#issuecomment-1375641514

   Still missing tests


-- 
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: issues-unsubscribe@commons.apache.org

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

[GitHub] [commons-net] marwenlahmar commented on pull request #127: Fixing proxy-problems in _parsePassiveModeReply

Posted by "marwenlahmar (via GitHub)" <gi...@apache.org>.
marwenlahmar commented on PR #127:
URL: https://github.com/apache/commons-net/pull/127#issuecomment-1564675862

   We are facing the same problem connecting to FTP servers behind a squid proxy, any chance for a new release ?


-- 
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: issues-unsubscribe@commons.apache.org

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

[GitHub] [commons-net] schmied commented on pull request #127: Fixing proxy-problems in _parsePassiveModeReply

Posted by GitBox <gi...@apache.org>.
schmied commented on PR #127:
URL: https://github.com/apache/commons-net/pull/127#issuecomment-1340714973

   This seems to fix my problem with squid and FTPS. By now, as a workaround, I set `org.apache.commons.net.ftp.ipAddressFromPasvResponse = true`.


-- 
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: issues-unsubscribe@commons.apache.org

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

[GitHub] [commons-net] garydgregory commented on pull request #127: Fixing proxy-problems in _parsePassiveModeReply

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #127:
URL: https://github.com/apache/commons-net/pull/127#issuecomment-1605570936

   > We are facing the same problem connecting to FTP servers behind a squid proxy, any chance for a new release ?
   
   Hi @marwenlahmar,
   Finally coming back to these parts...
   
   I am not comfortable merging this PR unless there is a test that shows that it fixes something. No matter what IRL testing may have taken place. 
   
   IOW, a test should fail without any change to the `main` tree. Then, when the `main` changes are applied, the test passes. 
   
   This new test can be set up in any way you want, using plain unit testing, using mock objects, or even Docker (there is a Maven docker plugin).
   
   HTH,
   G
   


-- 
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: issues-unsubscribe@commons.apache.org

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

[GitHub] [commons-net] DazzaL commented on pull request #127: Fixing proxy-problems in _parsePassiveModeReply

Posted by "DazzaL (via GitHub)" <gi...@apache.org>.
DazzaL commented on PR #127:
URL: https://github.com/apache/commons-net/pull/127#issuecomment-1674749212

   This 3.9 behaviour also breaks all FTP servers that sit behind F5 load balancers when passive mode is used which this patch fixes too as the client tries to connect to the F5 address Vs the serving pool member (connection refused error or read timeouts occur). Having said that isn't the resultant code now odd?
   As 3.9 added this isIpAddressFromPassiveResponse but now it's now set from the passive response anyway (and not overwritten from the control channel anymore). The only difference is that this setting still acts as a control switch for enabling / disabling the nat workaround strategy behaviour. So is this switch now not redundant here? Also what was the purpose of the switch, ie what's the problem with trusting the servers reply that resulted in a breaking change in 3.9.0 anyway?


-- 
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: issues-unsubscribe@commons.apache.org

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

[GitHub] [commons-net] SteveOswald commented on pull request #127: Fixing proxy-problems in _parsePassiveModeReply

Posted by GitBox <gi...@apache.org>.
SteveOswald commented on PR #127:
URL: https://github.com/apache/commons-net/pull/127#issuecomment-1339277659

   Hi @garydgregory ,
   
   we have tested it in our test environment as well as in our production environment. It works with and without proxy.
   
   Also, it does not comply with the RFC to simply have the pasvHost overwritten with the socket address every time (as is currently the case). The server specifies an address in the PASV response to connect to. This address should also be used. The only exception should be if the server sends an invalid address ala "0,0,0,0". This case is already covered in the code, so it is not necessary to always use the socket address - as is currently the case.
   
   A test case for this is factually hard, as it would require a working HTTP proxy. However, it follows from pure logic that the current behavior is wrong.


-- 
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: issues-unsubscribe@commons.apache.org

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

[GitHub] [commons-net] garydgregory commented on pull request #127: Fixing proxy-problems in _parsePassiveModeReply

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #127:
URL: https://github.com/apache/commons-net/pull/127#issuecomment-1339196589

   Hello @SteveOswald 
   This PR needs a test to prove the PR fixes something and to avoid regressions.


-- 
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: issues-unsubscribe@commons.apache.org

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