From 2bb61cb273cc057a34c91b3cbe33b971f2a3fde5 Mon Sep 17 00:00:00 2001 From: Kishore Rajasekar <86338791+ki1729@users.noreply.github.com> Date: Mon, 31 Oct 2022 08:38:46 -0700 Subject: [PATCH] Fixed servicebus default proxy configuration bug (#31832) * Fixing servicebus default proxy configuration bug * Updating changelog and tests --- .../azure-messaging-servicebus/CHANGELOG.md | 2 +- .../servicebus/ServiceBusClientBuilder.java | 47 +------------------ .../ServiceBusClientBuilderTest.java | 44 ++++++++--------- 3 files changed, 21 insertions(+), 72 deletions(-) diff --git a/sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md b/sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md index c9aa709ecad83..e22ea796545c8 100644 --- a/sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md +++ b/sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md @@ -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) diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusClientBuilder.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusClientBuilder.java index 4a7982f55c0c8..35ee837a92f7b 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusClientBuilder.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusClientBuilder.java @@ -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; @@ -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; @@ -745,7 +742,7 @@ private ConnectionOptions getConnectionOptions() { } if (proxyOptions == null) { - proxyOptions = getDefaultProxyConfiguration(configuration); + proxyOptions = ProxyOptions.fromConfiguration(configuration); } final CbsAuthorizationType authorizationType = credentials instanceof ServiceBusSharedKeyCredential @@ -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(); } diff --git a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusClientBuilderTest.java b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusClientBuilderTest.java index 0c0b084a08a45..260fcf8994ba4 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusClientBuilderTest.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusClientBuilderTest.java @@ -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 @@ -386,16 +380,16 @@ public void testConnectionWithAzureSasCredential() { private static Stream 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:password@sub.example.com:8080", true), - Arguments.of("https://username:password@sub.example.com", 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:password@sub.example.com:8080"), + Arguments.of("https://username:password@sub.example.com") ); }