socks proxy rebase #2571
socks proxy rebase #2571
Conversation
We don't want people to use this incomplete implementation believing the proxy will be used for all their connections. This could have serious privacy implications, e.g. when using Tor via SOCKS proxy. See also #980
|
Nice work! The unit tests seem to be failing now due to a syntax error, do you want to look into this? |
…nsportProvider.getTransport
|
Thanks! I believe I have solved the issue with the tests. |
| public static int getPgpInlineDialogCounter() { | ||
| return sPgpInlineDialogCounter; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
unnecessary whitespace changes
| @@ -203,6 +223,11 @@ public Socket createSocket(Socket socket, String host, int port, String clientCe | |||
|
|
|||
| setSniHost(socketFactory, sslSocket, host); | |||
|
|
|||
| if (!proxySettings.enabled && socket == null) { | |||
There was a problem hiding this comment.
I don't understand this change, and how the socket.connect methods below are all removed. Does the method return a connected socket now, or what?
There was a problem hiding this comment.
Yes I think it does. Because the proxied case has to return a connected socket, all the methods do. I think this is okay but the method name should indicate that this is the case.
|
Thanks for picking this up! I think we should try to merge this asap, to avoid collecting bitdust again. If we keep this feature flagged, it should be simple to ensure nothing breaks. |
|
The If someone wants to work on this it's probably easier to start fresh. I don't want any of the remote stores to have to know about a proxy server. We should be able to pass a Socket factory to the remote stores. All the SOCKS proxy handling code could live in the factory class. |
A rebase that brings the socks_proxy branch up to date with the master branch for easier future development. And this time the base is k9mail/master and there are 6 commits, not over 1000.