apache
NET-642 using execPROT on FTPSClients with Proxy Settings removes Pro…
do not reset proxy settings when re-setting the socket factory create method identical to open openDataConnection for FTPS where proxy is used and ssl socket is created from ssl context
Expecting a feedback. Also I'm not sure how to write a unit test here, any hints?
You can start by looking at FTPSClientTest. There is also the option of using Docker through a Maven plugin like org.testcontainers:testcontainers
Yes, Gary. I see the hint but I don't know when I will make time for this. Also in the defect NET-642, HTTPS is mention but my fix is only for FTPS... but it is a start.
@garydgregory Hi, my idea for unit test is to set the proxy in FTPSClientTest.loginClient() and then execute client.connect(...) and PROT command but I'm not sure which hosts and ports to use? For example in SocketClientFunctionalTest are defined hosts and ports but I cannot connect to them.
We are blocked because of this issue to migrate to Commons NET. Will it be merged ?
Hi, we are also blocked due to this issue and so far had to copy the entire FTP client to our project to get around it. We'll see if we can contribute the test, but I am not able to understand where the happy path unit test is either, so maybe it's better not to touch anything. A solution would be amazing!
Hi, sooo... this PR allow to resolve a bug declared in 2017... but it's not merged because it lacks a unit test to fail without the main changes ? Who would create a unit test with a socks proxy to prove it doesn't work as intended ?
I wonder if every fork are made to use this PR as i'll do. Thanks @YaniM for the PR :heart:
There is at least one comment that has not been addressed. This puts the PR on the back burner for me.
"Who would create a unit test with a socks proxy to prove it doesn't work as intended ?"
- the person who cares to avoid a regression in future code changes
- the person who knows mocking or cares enough to learn it
- the person who understands free open source software
On the one hand, I agree with everything you said.
On the other hand, i wonder why there is no test to prove it works in the first place. But maybe there is, and one case is just not covered ? In this case, maybe it's easier to implement the missing case.
"i wonder why there is no test to prove it works in the first place" Open source is no different than working in some companies, some people do work at different levels. I bet that if you did a git blame on that code, found the author, contacted them, you'd get some reply (maybe) like "it's too hard", "works for my setup", "I was busy" or any number of other reasons...