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

[BUG] com.azure.core.http.HttpClient resolves proxy's DNS name only once and caches the IP address forever regardless of TTL. #38963

Open
3 tasks done
lukolszewski opened this issue Feb 27, 2024 · 8 comments
Assignees
Labels
Azure.Core azure-core bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention This issue needs attention from Azure service team or SDK team

Comments

@lukolszewski
Copy link

Describe the bug
com.azure.core.http.HttpClient when built with proxy name set, appears to perform only one DNS to IP address resolution during the build(). Later only the IP address is used for the proxy disregarding DNS TTL. When the DNS IP of the proxy changes the client still tries to access the old one causing connectivity issues unless it is destroyed and recreated.

To Reproduce
Create a com.azure.core.http.HttpClient, set proxy configuration using DNS name that resolves to one or more IP addresses with DNS A records. Build the client and use it, then change one of the proxy's DNS records to point to another IP. It will still try to connect to the old proxy IP.

Code Snippet

From azure-sdk-for-java/blob/main/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java line starting at 264

nettyHttpClient = nettyHttpClient.doOnChannelInit((connectionObserver, channel, socketAddress) -> { if (shouldApplyProxy(socketAddress, nonProxyHostsPattern)) { channel.pipeline() .addFirst(NettyPipeline.ProxyHandler, new HttpProxyHandler(AddressUtils.replaceWithResolved(buildProxyOptions.getAddress()), handler, proxyChallengeHolder)); } });

Expected behavior
Repeated address resolutions either in accordance with the DNS TTL, or some configuration variable.

Setup (please complete the following information):

  • OS: [e.g. iOS] not OS specific
  • IDE: [e.g. IntelliJ] nod IDE specific
  • Library/Libraries: [e.g. com.azure:azure-core:1.44.1 (groupId:artifactId:version)] (also present in latest)
  • Java version: [e.g. 8] 11
  • App Server/Environment: [e.g. Tomcat, WildFly, Azure Function, Apache Spark, Databricks, IDE plugin or anything special] Jenkins
  • Frameworks: [e.g. Spring Boot, Micronaut, Quarkus, etc] N/A

Additional context
Another way to reproduce in the actual environment is:
Use software that uses azure-sdk-for-java for connectivity to azure that uses HttpClient with a proxy configuration. Change the DNS IP of the proxy. Observe communication being broken.

For example: There is a Jenkins plugin called azure-ad-plugin. That plugin uses com.azure.core.http.HttpClient as a deep dependency building it on server start. After proxy's DNS IP (or one of its IPs) changes it still attempts to connect to the old IP until entire server is restarted logging io.netty.channel.ConnectTimeoutException

Information Checklist
Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

  • Bug Description Added
  • Repro Steps Added
  • Setup information Added
@github-actions github-actions bot added Azure.Core azure-core Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-triage This issue needs the team to triage. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Feb 27, 2024
@joshfree joshfree added bug This issue requires a change to an existing behavior in the product in order to be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that needs-team-triage This issue needs the team to triage. labels Feb 27, 2024
@github-actions github-actions bot added the needs-team-attention This issue needs attention from Azure service team or SDK team label Feb 27, 2024
@joshfree
Copy link
Member

Thank you for reporting this issue, @lukolszewski. @alzimmermsft could you take a look at fixing this TTL problem?

@alzimmermsft
Copy link
Member

Thanks for filing this issue @lukolszewski.

Have a quick question before I dive further into this. Does the proxy that you're using have an authentication requirement? If it does, could you check what happens if that is disabled for a test run that replicates the same flow when using proxy authentication.

@alzimmermsft alzimmermsft added needs-author-feedback More information is needed from author to address the issue. and removed needs-team-attention This issue needs attention from Azure service team or SDK team labels Feb 28, 2024
Copy link

Hi @lukolszewski. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

@lukolszewski
Copy link
Author

Thanks for filing this issue @lukolszewski.

Have a quick question before I dive further into this. Does the proxy that you're using have an authentication requirement? If it does, could you check what happens if that is disabled for a test run that replicates the same flow when using proxy authentication.

Hi, Thank you for looking into this. Our proxy doesn't require authentication (it is authenticated based on the client IP). I hope this answers the question.

@github-actions github-actions bot added needs-team-attention This issue needs attention from Azure service team or SDK team and removed needs-author-feedback More information is needed from author to address the issue. labels Mar 5, 2024
@alzimmermsft
Copy link
Member

Thanks for filing this issue @lukolszewski.
Have a quick question before I dive further into this. Does the proxy that you're using have an authentication requirement? If it does, could you check what happens if that is disabled for a test run that replicates the same flow when using proxy authentication.

Hi, Thank you for looking into this. Our proxy doesn't require authentication (it is authenticated based on the client IP). I hope this answers the question.

Thanks @lukolszewski, this did answer my question.

Given there isn't authentication for the proxy this ends up using the handling provided by Reactor Netty:

https://github.com/Azure/azure-sdk-for-java/blob/main/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java#L273

Which may be showing an issue within Reactor Netty itself. Unless through debugging you're seeing this call into the code path we've added for proxy handling when there is authentication for the proxy.

@alzimmermsft alzimmermsft added needs-author-feedback More information is needed from author to address the issue. and removed needs-team-attention This issue needs attention from Azure service team or SDK team labels Mar 25, 2024
Copy link

Hi @lukolszewski. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

@lukolszewski
Copy link
Author

Thanks for filing this issue @lukolszewski.
Have a quick question before I dive further into this. Does the proxy that you're using have an authentication requirement? If it does, could you check what happens if that is disabled for a test run that replicates the same flow when using proxy authentication.

Hi, Thank you for looking into this. Our proxy doesn't require authentication (it is authenticated based on the client IP). I hope this answers the question.

Thanks @lukolszewski, this did answer my question.

Given there isn't authentication for the proxy this ends up using the handling provided by Reactor Netty:

https://github.com/Azure/azure-sdk-for-java/blob/main/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java#L273

Which may be showing an issue within Reactor Netty itself. Unless through debugging you're seeing this call into the code path we've added for proxy handling when there is authentication for the proxy.

Perhaps I'm misunderstanding, are you saying the problem is not with azure-sdk, but possibly with Reactor Netty, because the code I mentioned is called only when authentication is used?

  • when HTTP auth is used "handler" is defined and only then AddressUtils.replaceWithResolved is used replacing the DNS name with the current IP - when IP changes later (and DNS name is updated) this never gets updated.
  • otherwise line 274 is executed setting the DNS name as the proxy address in properties.

If so, I understand how this issue in line 268 new HttpProxyHandler(AddressUtils.replaceWithResolved(buildProxyOptions.getAddress()), can only affect users of HTTP authenticated proxies.

However, we are experiencing the issue too with no HTTP auth. So the question is, is it a bug in Reactor Netty's HttpClient or NettyAsyncHttpClientBuilder? The fact is it impacts the use of the azure sdk (including with no proxy auth). Perhaps next week I'll have more time to trace this, but at this time I'm unable to find exactly how this proxy is being used. I can see it is set as a property of com.azure.core.http.NettyAsyncHttpClient, but I haven't found how exactly is it used yet.

There is also a question (if indeed this is a "feature/bug" of Reacttor Netty) if Reactor Netty is supposed to refresh DNS resolution for long lived HttpClients. Perhaps the consumer of the library would be better off implementing ability to refresh the client periodically?

@github-actions github-actions bot added needs-team-attention This issue needs attention from Azure service team or SDK team and removed needs-author-feedback More information is needed from author to address the issue. labels Mar 29, 2024
@lukolszewski
Copy link
Author

To better illustrate the issue I created a very simple demo app (attached in the zip)
issue.zip The code of the main class is shown below.

The app simply creates a http client using azure-core-http-netty that uses a non-authenticated http proxy. To illustrate the problem I suggest to use local squid proxy setup with docker (or any other proxy you have access to). Then modify your /etc/hosts to include "X.X.X.X myproxy" where X.X.X.X is the proxy IP.

Then let the app run. It will attempt to fetch from the web via the proxy every 5s. Leave it running and in another terminal open your /etc/hosts and change the line "X.X.X.X myproxy" to "Y.Y.Y.Y myproxy" where Y.Y.Y.Y is a non existing IP.

Observe the app still fetching the page using the old proxy IP.
It is expected it will stop working after /etc/hosts (or a DNS A record which it simulates) updates the proxy IP to a wrong value.

Also, with the bad IP in /etc/hosts. Stop the app and run it again. It will display that it cannot connect to the proxy every 5 seconds. Leave it running and update /etc/hosts to the correct IP.

Observe the app still unable to connect and attempting to contact the bad IP despite the DNS update.
It is expected the that the app starts to work and connects successfully.

IMO this DNS/IP refreshing is not really a job of the low level connectivity library (Netty itself) but the Azure SDK that uses it is probably a place where a DNS change of a proxy A record shouldn't cause a persistent failure to connect.

What do you think?

Here is the code of the main class of the tiny app attached:

package com.example;
import com.azure.core.http.HttpClient;
import com.azure.core.http.netty.NettyAsyncHttpClientBuilder;
import com.azure.core.http.HttpRequest;
import com.azure.core.http.HttpMethod;
import com.azure.core.http.ProxyOptions;
import java.net.InetSocketAddress;

public class App {
    public static void main(String[] args) {
        // Set up the proxy configuration
        ProxyOptions proxyOptions = new ProxyOptions(ProxyOptions.Type.HTTP, new InetSocketAddress("myproxy", 3128));

        // Create the HttpClient using the NettyAsyncHttpClientBuilder and configure the proxy
        HttpClient client = new NettyAsyncHttpClientBuilder()
            .proxy(proxyOptions)
            .build();

        // Infinite loop to continuously send requests
        while (true) {
            HttpRequest request = new HttpRequest(HttpMethod.GET, "http://httpbin.org/get");
            client.send(request).subscribe(response -> {
                response.getBodyAsString().subscribe(
                    System.out::println,
                    error -> System.err.println("Error retrieving the response: " + error.getMessage())
                );
            }, error -> {
                System.err.println("Error sending HTTP request: " + error.getMessage());
            });

            try {
                Thread.sleep(5000); // Wait 5 seconds before sending the next request
            } catch (InterruptedException e) {
                System.err.println("Thread interrupted: " + e.getMessage());
                break; // Exit the loop if the thread is interrupted
            }
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core azure-core bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention This issue needs attention from Azure service team or SDK team
Projects
Status: Planned
Development

No branches or pull requests

3 participants