Skip to content

Commit e576c52

Browse files
seratchseratch
authored
Improve the error caused in the #792 scenario (#807)
* Improve the error caused in the #792 scenario * Fix the test
1 parent 2fb3e82 commit e576c52

File tree

5 files changed

+144
-14
lines changed

5 files changed

+144
-14
lines changed

‎slack-api-client/src/main/java/com/slack/api/Slack.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public String issueSocketModeUrl(String appToken) throws IOException {
173173
} catch (SlackApiException e) {
174174
String message = "Failed to connect to the Socket Mode API endpoint. (" +
175175
"status: " + e.getResponse().code() + ", " +
176-
"error: " + e.getError().getError() +
176+
"error: " + (e.getError() != null ? e.getError().getError() : "") +
177177
")";
178178
throw new IOException(message, e);
179179
}

‎slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientJavaWSImpl.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,17 +219,29 @@ public UnderlyingWebSocketSession(URI serverUri, Map<String, String> httpHeaders
219219
// FIXME: the proxy settings here may not work
220220
SlackConfig slackConfig = smc.getSlack().getHttpClient().getConfig();
221221
Map<String, String> proxyHeaders = slackConfig.getProxyHeaders();
222+
if (proxyHeaders == null) {
223+
proxyHeaders = new HashMap<>();
224+
}
225+
222226
String proxyUrl = slackConfig.getProxyUrl();
223227
if (proxyUrl != null) {
224228
if (smc.getLogger().isDebugEnabled()) {
225229
smc.getLogger().debug("The SocketMode client's going to use an HTTP proxy: {}", proxyUrl);
226230
}
227231
ProxyUrlUtil.ProxyUrl parsedProxy = ProxyUrlUtil.parse(proxyUrl);
232+
if (parsedProxy.getUsername() != null && parsedProxy.getPassword() != null) {
233+
// see also: https://github.com/slackapi/java-slack-sdk/issues/792#issuecomment-895961176
234+
String message = "Unfortunately, " +
235+
"having username:password with the Java-WebSocket library is not yet supported. " +
236+
"Consider using other implementations such SocketModeClient.Backend.Tyrus.";
237+
throw new UnsupportedOperationException(message);
238+
}
239+
228240
InetSocketAddress proxyAddress = new InetSocketAddress(parsedProxy.getHost(), parsedProxy.getPort());
229241
this.setProxy(new Proxy(Proxy.Type.HTTP, proxyAddress));
230242
ProxyUrlUtil.setProxyAuthorizationHeader(proxyHeaders, parsedProxy);
231243
}
232-
if (slackConfig.getProxyHeaders() != null) {
244+
if (proxyHeaders != null && !proxyHeaders.isEmpty()) {
233245
for (Map.Entry<String, String> each : proxyHeaders.entrySet()) {
234246
this.addHeader(each.getKey(), each.getValue());
235247
}

‎slack-api-client/src/main/java/com/slack/api/util/http/ProxyUrlUtil.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,14 @@ public static ProxyUrl parse(String proxyUrl) {
4848
.username(userAndPassword[0])
4949
.password(userAndPassword[1])
5050
.host(hostAndPort[0])
51-
.port(hostAndPort.length == 2 ? Integer.parseInt(hostAndPort[1]) : 80)
51+
.port(hostAndPort.length == 2 ? Integer.parseInt(hostAndPort[1].replace("/", "")) : 80)
5252
.build();
5353
} else {
5454
String[] hostAndPort = proxyUrl.split("://")[1].split(":");
5555
return ProxyUrl.builder()
5656
.schema(schema)
5757
.host(hostAndPort[0])
58-
.port(hostAndPort.length == 2 ? Integer.parseInt(hostAndPort[1]) : 80)
58+
.port(hostAndPort.length == 2 ? Integer.parseInt(hostAndPort[1].replace("/", "")) : 80)
5959
.build();
6060
}
6161
}

‎slack-api-client/src/test/java/test_locally/api/socket_mode/SocketModeClientTest.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,16 @@ public void connect() throws Exception {
6868
try (SocketModeClient client = slack.socketMode(VALID_APP_TOKEN)) {
6969
AtomicBoolean received = new AtomicBoolean(false);
7070
client.addWebSocketMessageListener(helloListener(received));
71-
client.addWebSocketErrorListener(error -> {});
72-
client.addWebSocketCloseListener((code, reason) -> {});
73-
client.addEventsApiEnvelopeListener(envelope -> {});
74-
client.addInteractiveEnvelopeListener(envelope -> {});
75-
client.addSlashCommandsEnvelopeListener(envelope -> {});
71+
client.addWebSocketErrorListener(error -> {
72+
});
73+
client.addWebSocketCloseListener((code, reason) -> {
74+
});
75+
client.addEventsApiEnvelopeListener(envelope -> {
76+
});
77+
client.addInteractiveEnvelopeListener(envelope -> {
78+
});
79+
client.addSlashCommandsEnvelopeListener(envelope -> {
80+
});
7681

7782
client.connect();
7883
Thread.sleep(500L);
@@ -163,11 +168,16 @@ public void connect_JavaWebSocket() throws Exception {
163168
try (SocketModeClient client = slack.socketMode(VALID_APP_TOKEN, SocketModeClient.Backend.JavaWebSocket)) {
164169
AtomicBoolean received = new AtomicBoolean(false);
165170
client.addWebSocketMessageListener(helloListener(received));
166-
client.addWebSocketErrorListener(error -> {});
167-
client.addWebSocketCloseListener((code, reason) -> {});
168-
client.addEventsApiEnvelopeListener(envelope -> {});
169-
client.addInteractiveEnvelopeListener(envelope -> {});
170-
client.addSlashCommandsEnvelopeListener(envelope -> {});
171+
client.addWebSocketErrorListener(error -> {
172+
});
173+
client.addWebSocketCloseListener((code, reason) -> {
174+
});
175+
client.addEventsApiEnvelopeListener(envelope -> {
176+
});
177+
client.addInteractiveEnvelopeListener(envelope -> {
178+
});
179+
client.addSlashCommandsEnvelopeListener(envelope -> {
180+
});
171181

172182
client.connect();
173183
Thread.sleep(500L);

‎slack-api-client/src/test/java/test_locally/api/socket_mode/SocketModeClient_Proxies_Test.java‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package test_locally.api.socket_mode;
2+
3+
import com.slack.api.Slack;
4+
import com.slack.api.SlackConfig;
5+
import com.slack.api.socket_mode.SocketModeClient;
6+
import lombok.extern.slf4j.Slf4j;
7+
import okhttp3.Credentials;
8+
import org.eclipse.jetty.proxy.ConnectHandler;
9+
import org.eclipse.jetty.proxy.ProxyServlet;
10+
import org.eclipse.jetty.server.Request;
11+
import org.eclipse.jetty.server.Server;
12+
import org.eclipse.jetty.server.ServerConnector;
13+
import org.eclipse.jetty.servlet.ServletContextHandler;
14+
import org.eclipse.jetty.servlet.ServletHolder;
15+
import org.junit.After;
16+
import org.junit.Before;
17+
import org.junit.Test;
18+
import test_with_remote_apis.AuthProxyHeadersTest;
19+
import util.PortProvider;
20+
import util.socket_mode.MockWebApiServer;
21+
22+
import javax.servlet.ServletException;
23+
import javax.servlet.http.HttpServletRequest;
24+
import javax.servlet.http.HttpServletResponse;
25+
import java.io.IOException;
26+
import java.util.Collections;
27+
import java.util.HashMap;
28+
import java.util.Locale;
29+
import java.util.Map;
30+
import java.util.concurrent.atomic.AtomicInteger;
31+
32+
@Slf4j
33+
public class SocketModeClient_Proxies_Test {
34+
35+
static final String VALID_APP_TOKEN = "xapp-valid-123123123123123123123123123123123123";
36+
37+
MockWebApiServer webApiServer = new MockWebApiServer();
38+
SlackConfig config = new SlackConfig();
39+
40+
static Server proxyServer = new Server();
41+
static ServerConnector proxyServerConnector = new ServerConnector(proxyServer);
42+
static Integer proxyServerPort;
43+
44+
AtomicInteger proxyCallCount = new AtomicInteger(0);
45+
46+
@Before
47+
public void setUp() throws Exception {
48+
webApiServer.start();
49+
config.setMethodsEndpointUrlPrefix(webApiServer.getMethodsEndpointPrefix());
50+
51+
// https://github.com/eclipse/jetty.project/blob/jetty-9.2.30.v20200428/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ProxyServer.java
52+
proxyServerPort = PortProvider.getPort(AuthProxyHeadersTest.class.getName());
53+
proxyServerConnector.setPort(proxyServerPort);
54+
proxyServer.addConnector(proxyServerConnector);
55+
ConnectHandler proxy = new ConnectHandler() {
56+
@Override
57+
public void handle(String target, Request br, HttpServletRequest request, HttpServletResponse res)
58+
throws ServletException, IOException {
59+
log.info("Proxy server handles a new connection (target: {})", target);
60+
super.handle(target, br, request, res);
61+
if (res.getStatus() != 407) {
62+
proxyCallCount.incrementAndGet();
63+
}
64+
}
65+
66+
@Override
67+
protected boolean handleAuthentication(HttpServletRequest request, HttpServletResponse response, String address) {
68+
for (String name : Collections.list(request.getHeaderNames())) {
69+
log.info("{}: {}", name, request.getHeader(name));
70+
if (name.toLowerCase(Locale.ENGLISH).equals("proxy-authorization")) {
71+
return request.getHeader(name).equals("Basic bXktdXNlcm5hbWU6bXktcGFzc3dvcmQ=");
72+
}
73+
}
74+
return false;
75+
}
76+
};
77+
proxyServer.setHandler(proxy);
78+
ServletContextHandler context = new ServletContextHandler(proxy, "/", ServletContextHandler.SESSIONS);
79+
ServletHolder proxyServlet = new ServletHolder(ProxyServlet.class);
80+
context.addServlet(proxyServlet, "/*");
81+
proxyServer.start();
82+
83+
config.setProxyUrl("http://127.0.0.1:" + proxyServerPort);
84+
85+
Map<String, String> proxyHeaders = new HashMap<>();
86+
String username = "my-username";
87+
String password = "my-password";
88+
proxyHeaders.put("Proxy-Authorization", Credentials.basic(username, password));
89+
config.setProxyHeaders(proxyHeaders);
90+
}
91+
92+
@After
93+
public void tearDown() throws Exception {
94+
webApiServer.stop();
95+
96+
proxyServer.removeConnector(proxyServerConnector);
97+
proxyServer.stop();
98+
}
99+
100+
@Test(expected = UnsupportedOperationException.class)
101+
public void proxyAuth() throws Exception {
102+
SlackConfig config = new SlackConfig();
103+
config.setMethodsEndpointUrlPrefix(webApiServer.getMethodsEndpointPrefix());
104+
config.setProxyUrl("http://my-username:my-password@localhost:" + proxyServerPort + "/");
105+
Slack slack = Slack.getInstance(config);
106+
slack.socketMode(VALID_APP_TOKEN, SocketModeClient.Backend.JavaWebSocket);
107+
}
108+
}

0 commit comments

Comments
 (0)