-
Notifications
You must be signed in to change notification settings - Fork 914
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
ARTEMIS-1194: SOCKS proxy support #1293
Conversation
nice feature!!! = it needs a test.. (especially one that uses the URI for the connector) and... how to test this.. any proxy support in Java we can deploy? |
Also needs a JIRA and there are a handful of checkstyle violations. |
@@ -177,6 +181,18 @@ | |||
// will be handled by the server's http server. | |||
private boolean httpUpgradeEnabled; | |||
|
|||
private boolean socksEnabled; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
proxyEnabled is a more intuite name...
especially if you specify a connector with ?useSocket=true;proxyHost=;proxyPort;proxyUser;proxyPassword...
so.. better property names:
- proxyEnabled
- proxyUser
- proxyPassword
- proxyPort
- proxyHost
Please open a JIRA for this, squash all of your commits, and amend your commit message to follow what's outlined in the Hacking Guide. |
Will do 👍 |
you could squash all your committs into a single one here: git rebase -i HEAD~2 squash them, change the tittlegit push origi -f |
this is ready for another look @clebertsuconic |
I was trying to make a test like this: but I'm not sure I will have to time to finish it... if you want to take over this test? |
can do, so the point of this test is just to check that NettyConnector can establish a connection when the proxy handler is enabled, right? |
Yes, basically I wanted to start a proxy, and make sure it worked through the proxy. |
I tried it with the MockServer proxy but it seems to only work with http traffic.. Maybe it's sufficient to just open a socket at the socks port and make sure a connection gets established? Handling the SOCKS protocol is technically the responsibility of Netty. |
@adagys we still need a way to test it though.. I looked at their website and it seems to support SOCKS.. but I didn't figure out how. I think we should remove the option you added to ignore localhost though. If it's proxy, always proxy... |
@clebertsuconic @adagys This has been open since May. What needs to happen before we can merge/close? |
Test |
I couldn't get it to work in the end and don't have time to work on it anymore.., can be closed |
@adagys did you at least try the feature alone? was it working? I couldn't get it to work. I was interested on the feature.. but I couldn't get the test to work. |
Yep, I remember seeing it work in an end-to-end scenario, but couldn't get the test to work either.. Which is strange |
Can you do one last rebase 👍 and I will work on the tests soon. (kind of last week) |
Add a Netty socks proxy handler during channel initialisation to allow Artemis to communicate via a SOCKS proxy. Supports SOCKS version 4a & 5. Even if enabled in configuration, the proxy will not be used when the target host is a loopback address.
Rebased 👍 |
@clebertsuconic whats occuring on this, we ok to merge? And simply mark feature beta until some decent tests? |
@clebertsuconic I'm merging this. |
just needs sorting dependencies, seems the project removed netty all here, so just having to add
|
@michaelandrepearce, |
@michaelandrepearce, I think I've got a fix for this. Will send a PR shortly... |
Just sent #3010. |
Map<String, Object> params = new HashMap<>();
params.put(TransportConstants.HOST_PROP_NAME, "localhost");
params.put(TransportConstants.PORT_PROP_NAME, "61617"); //<acceptor name="netty">tcp://0.0.0.0:61617</acceptor>
params.put(TransportConstants.PROXY_ENABLED_PROP_NAME, "true");
params.put(TransportConstants.PROXY_HOST_PROP_NAME, "localhost");
params.put(TransportConstants.PROXY_PORT_PROP_NAME, "1080");
params.put(TransportConstants.PROXY_USERNAME_PROP_NAME, "user");
params.put(TransportConstants.PROXY_PASSWORD_PROP_NAME, "WRONG_PASSWORD");
TransportConfiguration transportConfiguration = new TransportConfiguration("org.apache.activemq.artemis.core.remoting.impl.netty.NettyConnectorFactory", params);
ActiveMQConnectionFactory activeMQConnectionFactory = ActiveMQJMSClient.createConnectionFactoryWithoutHA(JMSFactoryType.CF, transportConfiguration); <dependency>
<groupId>org.apache.activemq</groupId>
<artifactId>artemis-jms-client</artifactId>
<version>2.14.0</version>
</dependency> This is ignoring the proxy conf. Am I doing something wrong? |
@celiovasconcelos, this is an old, closed pull request. It's not really meant for support. Try using the ActiveMQ User Mailing List. |
Sorry. |
This work enables Artemis connectors to use a SOCKS proxy, by adding an appropriate netty socks handler to the channel pipeline. The proxy is only enabled for a public remote host.
I tested that it works correctly.
If this is acceptable functionality to be included in Artemis I'm happy to do some more polishing to get it in the right shape.