Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QFJ-285 Add proxy support (compatible with SSL) #91

Merged
merged 3 commits into from Dec 13, 2016

Conversation

soliad
Copy link
Contributor

@soliad soliad commented Dec 13, 2016

This patch add support for proxy for initiator connections.
It's tested and working with and without SSL

@chrjohn
Copy link
Member

chrjohn commented Dec 13, 2016

Hi @soliad ,
thanks for the PR. Do you happen to have some time to put the new settings into the configuration.html page?
If not, no problem. Then I will do it later.

Best regards,
Chris.

@soliad
Copy link
Contributor Author

soliad commented Dec 13, 2016

Sure I'll take a look

@chrjohn
Copy link
Member

chrjohn commented Dec 13, 2016

Great, I'll see if I can still fit it into QFJ 1.6.3 which is going to be released before end of the year.

@chrjohn chrjohn added this to the QFJ 1.6.3 milestone Dec 13, 2016
@soliad
Copy link
Contributor Author

soliad commented Dec 13, 2016

Hi @chrjohn
I've added the settings in configuration.html and fixed a type for HTTP proxy (we tested it with socks with and without SSL)

I'm not sure if it will be easy to merge to QFJ 1.6 with all the SSL changes around.

Regards
Arnaud

@soliad
Copy link
Contributor Author

soliad commented Dec 13, 2016

My bad the branch 1.6 has all those SSL changes, so merging should be simple. (I did not see 1.6.x restarted from the master in October)

@chrjohn
Copy link
Member

chrjohn commented Dec 13, 2016

Hi @soliad ,

thanks for the doc update. Good catch on the proxy user/password typo. :)

Cheers,
Chris.

@chrjohn chrjohn merged commit 47a0c78 into quickfix-j:master Dec 13, 2016
chrjohn added a commit that referenced this pull request Dec 13, 2016
QFJ-285 Add proxy support (compatible with SSL)
(cherry picked from commit 47a0c78)
@xiaodwy
Copy link

xiaodwy commented Dec 27, 2016

Hello @soliad,

Thanks for your update of the proxy feature, I have tested it and found the proxy does not compatible with the existing reconnecting logic of the initiator.
If the initiator failed to connect to the current fix server, it will try to reconnect to the next one with some intervals. But if I enable the proxy feature, the retrial will never success because the destination address of ProxyRequest would never be updated since the ProxyRequest was bound to a ProxyConnector which is reused for each reconnecting attempt.

Here's my configuration file of the initiator(I use quickfixj-examples/banzai for testing) with proxy settings and multiple FIX server addresses:

  1. The first address ip:1111 is not in listening state, so we should get a TCP connect failure.
  2. The second one is an actived echo server, I add it to test the proxy with a none-FIX server, so we should get a FIX error(without SSL) or a SSL error(with SSL).
  3. The third address is a good one.
[default]
FileStorePath=target/data/banzai
ConnectionType=initiator
SenderCompID=BANZAI
TargetCompID=EXEC
SocketConnectHost=localhost
StartTime=00:00:00
EndTime=00:00:00
HeartBtInt=5
ReconnectInterval=5

[session]
BeginString=FIX.4.4
# test reconnect with wrong address: port 1111 is down
SocketConnectHost=172.31.1.55
SocketConnectPort=1111
# test reconnect with wrong address: port 50000 is an echo server(which is not a fix server)
SocketConnectHost1=172.31.1.55
SocketConnectPort1=50000
# test reconnect with correct address:
SocketConnectHost2=172.31.1.55
SocketConnectPort2=9880
SocketUseSSL=N
######## proxy settings for session #######
ProxyType=socks
ProxyVersion=5
ProxyHost=172.31.1.55
ProxyPort=8889
ProxyUser=testuser
ProxyPassword=password

Could you take a look?
Let me know if you need more information :)

Regards,
Dong

@xiaodwy
Copy link

xiaodwy commented Dec 27, 2016

Hi @soliad ,

I also tested the HTTP proxy with/without reconnecting to the server, but it didn't work for me. I debugged and found an issue with the code.
If you create the HttpProxyRequest this way:

        HttpProxyRequest req = new HttpProxyRequest(uri); // uri is a String

you will get a HTTP GET request, but we need a HTTP CONNECT request instead, so maybe we need to instance the HttpProxyRequest like this:

         HttpProxyRequest req = new HttpProxyRequest(address); // address is an InetSocketAddress

You can find the api doc of HttpProxyRequest here: https://mina.apache.org/mina-project/apidocs/
image

Here's my config file:

[default]
FileStorePath=target/data/banzai
ConnectionType=initiator
SenderCompID=BANZAI
TargetCompID=EXEC
SocketConnectHost=localhost
StartTime=00:00:00
EndTime=00:00:00
HeartBtInt=5
ReconnectInterval=5

[session]
BeginString=FIX.4.4
# a good FIX server address:
SocketConnectHost2=172.31.1.55
SocketConnectPort2=9880
SocketUseSSL=N
######## proxy settings for session #######
ProxyType=http
ProxyVersion=1.1
ProxyHost=172.31.1.55
ProxyPort=3128
ProxyUser=testuser
ProxyPassword=password

Thanks,
Dong

@soliad
Copy link
Contributor Author

soliad commented Dec 29, 2016

Hi @xiaodwy

I'll take a look for the reconnect issue.
For the HTTP CONNECT, did you try with
HttpProxyRequest req = new HttpProxyRequest(address); // address is an InetSocketAddress ?

Arnaud

@soliad
Copy link
Contributor Author

soliad commented Dec 29, 2016

Looks like an easy fix, doing some tests and hoping to do a new PR tomorrow.

@xiaodwy
Copy link

xiaodwy commented Dec 30, 2016

Hi @soliad ,

For the HTTP CONNECT, did you try with
HttpProxyRequest req = new HttpProxyRequest(address); // address is an InetSocketAddress ?

Actually I didn't try the fix with your patch, so sorry... anything wrong?
I found the HTTP proxy does not work(SOCKS proxy is good), so I captured the TCP connection to debug the issue. I noticed that the proxy request was GET(which is not correct), so I reviewed the code and told you my findings :)

Thanks,
Dong

@soliad
Copy link
Contributor Author

soliad commented Dec 30, 2016

@xiaodwy, I put a new PR #92 that should fix those 2 issues.

@xiaodwy
Copy link

xiaodwy commented Jan 3, 2017

Comment moved to #92.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants