Skip to content

Commit

Permalink
Fixed servicebus default proxy configuration bug (Azure#31832)
Browse files Browse the repository at this point in the history
* Fixing servicebus default proxy configuration bug

* Updating changelog and tests
  • Loading branch information
ki1729 authored Oct 31, 2022
1 parent 1fd4c2f commit 2bb61cb
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 72 deletions.
2 changes: 1 addition & 1 deletion sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
### Breaking Changes

### Bugs Fixed
- Fixed `listQueues`, `listTopics`, `listRules`, `listSubscriptions`, `createQueue` and `createSubscriptionWithResponse` in `ServiceBusAdministrationClient` class. ([#31712](https://github.com/Azure/azure-sdk-for-java/issues/31712))
- Fixed incorrect proxy configuration using environment variables. ([24230](https://github.com/Azure/azure-sdk-for-java/issues/24230))
### Other Changes

## 7.12.1 (2022-10-25)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import com.azure.core.amqp.AmqpClientOptions;
import com.azure.core.amqp.AmqpRetryOptions;
import com.azure.core.amqp.AmqpTransportType;
import com.azure.core.amqp.ProxyAuthenticationType;
import com.azure.core.amqp.ProxyOptions;
import com.azure.core.amqp.client.traits.AmqpTrait;
import com.azure.core.amqp.implementation.AzureTokenManagerProvider;
Expand Down Expand Up @@ -53,9 +52,7 @@
import reactor.core.scheduler.Scheduler;
import reactor.core.scheduler.Schedulers;

import java.net.InetSocketAddress;
import java.net.MalformedURLException;
import java.net.Proxy;
import java.net.URL;
import java.time.Duration;
import java.util.Locale;
Expand Down Expand Up @@ -745,7 +742,7 @@ private ConnectionOptions getConnectionOptions() {
}

if (proxyOptions == null) {
proxyOptions = getDefaultProxyConfiguration(configuration);
proxyOptions = ProxyOptions.fromConfiguration(configuration);
}

final CbsAuthorizationType authorizationType = credentials instanceof ServiceBusSharedKeyCredential
Expand All @@ -770,48 +767,6 @@ private ConnectionOptions getConnectionOptions() {
}
}

private ProxyOptions getDefaultProxyConfiguration(Configuration configuration) {
ProxyAuthenticationType authentication = ProxyAuthenticationType.NONE;
if (proxyOptions != null) {
authentication = proxyOptions.getAuthentication();
}

String proxyAddress = configuration.get(Configuration.PROPERTY_HTTP_PROXY);

if (CoreUtils.isNullOrEmpty(proxyAddress)) {
return ProxyOptions.SYSTEM_DEFAULTS;
}

return getProxyOptions(authentication, proxyAddress, configuration,
Boolean.parseBoolean(configuration.get("java.net.useSystemProxies")));
}

private ProxyOptions getProxyOptions(ProxyAuthenticationType authentication, String proxyAddress,
Configuration configuration, boolean useSystemProxies) {
String host;
int port;
if (HOST_PORT_PATTERN.matcher(proxyAddress.trim()).find()) {
final String[] hostPort = proxyAddress.split(":");
host = hostPort[0];
port = Integer.parseInt(hostPort[1]);
final Proxy proxy = new Proxy(Proxy.Type.HTTP, new InetSocketAddress(host, port));
final String username = configuration.get(ProxyOptions.PROXY_USERNAME);
final String password = configuration.get(ProxyOptions.PROXY_PASSWORD);
return new ProxyOptions(authentication, proxy, username, password);
} else if (useSystemProxies) {
// java.net.useSystemProxies needs to be set to true in this scenario.
// If it is set to false 'ProxyOptions' in azure-core will return null.
com.azure.core.http.ProxyOptions coreProxyOptions = com.azure.core.http.ProxyOptions
.fromConfiguration(configuration);
return new ProxyOptions(authentication, new Proxy(coreProxyOptions.getType().toProxyType(),
coreProxyOptions.getAddress()), coreProxyOptions.getUsername(), coreProxyOptions.getPassword());
} else {
LOGGER.verbose("'HTTP_PROXY' was configured but ignored as 'java.net.useSystemProxies' wasn't "
+ "set or was false.");
return ProxyOptions.SYSTEM_DEFAULTS;
}
}

private static boolean isNullOrEmpty(String item) {
return item == null || item.isEmpty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,26 +234,20 @@ void invalidPrefetch() {

@MethodSource("getProxyConfigurations")
@ParameterizedTest
public void testProxyOptionsConfiguration(String proxyConfiguration, boolean expectedClientCreation) {
public void testProxyOptionsConfiguration(String proxyConfiguration) {
Configuration configuration = TestUtils.getGlobalConfiguration().clone();
configuration
.put(Configuration.PROPERTY_HTTP_PROXY, proxyConfiguration)
.put(JAVA_NET_USER_SYSTEM_PROXIES, "true");
boolean clientCreated = false;
try {
ServiceBusReceiverClient syncClient = new ServiceBusClientBuilder()
.connectionString(NAMESPACE_CONNECTION_STRING)
.configuration(configuration)
.receiver()
.topicName("baz").subscriptionName("bar")
.receiveMode(ServiceBusReceiveMode.PEEK_LOCK)
.buildClient();

clientCreated = true;
} catch (Exception ex) {
}

Assertions.assertEquals(expectedClientCreation, clientCreated);
// Client creation should not fail with incorrect proxy configuration
ServiceBusReceiverClient syncClient = new ServiceBusClientBuilder()
.connectionString(NAMESPACE_CONNECTION_STRING)
.configuration(configuration)
.receiver()
.topicName("baz").subscriptionName("bar")
.receiveMode(ServiceBusReceiveMode.PEEK_LOCK)
.buildClient();
}

@Test
Expand Down Expand Up @@ -386,16 +380,16 @@ public void testConnectionWithAzureSasCredential() {

private static Stream<Arguments> getProxyConfigurations() {
return Stream.of(
Arguments.of("http://localhost:8080", true),
Arguments.of("localhost:8080", true),
Arguments.of("localhost_8080", false),
Arguments.of("http://example.com:8080", true),
Arguments.of("http://sub.example.com:8080", true),
Arguments.of(":8080", false),
Arguments.of("http://localhost", true),
Arguments.of("sub.example.com:8080", true),
Arguments.of("https://username:[email protected]:8080", true),
Arguments.of("https://username:[email protected]", true)
Arguments.of("http://localhost:8080"),
Arguments.of("localhost:8080"),
Arguments.of("localhost_8080"),
Arguments.of("http://example.com:8080"),
Arguments.of("http://sub.example.com:8080"),
Arguments.of(":8080"),
Arguments.of("http://localhost"),
Arguments.of("sub.example.com:8080"),
Arguments.of("https://username:[email protected]:8080"),
Arguments.of("https://username:[email protected]")
);

}
Expand Down

0 comments on commit 2bb61cb

Please sign in to comment.