Add HTTP/HTTPS proxy support to client configuration

[dhufe]

Motivation

When trying to use taskwarrior behind an HTTP proxy, I noticed that proxy support was missing. This PR adds the necessary functionality to enable taskwarrior to work in proxy-required environments.

Summary

This PR adds support for HTTP/HTTPS proxies to the client configuration, allowing requests to be routed through proxy servers based on standard environment variables.
Changes

Proxy Support Implementation: Updated the HTTP client to automatically detect and use proxies configured via HTTP_PROXY and HTTPS_PROXY environment variables
Comprehensive Testing: Added extensive test coverage to validate various proxy configuration scenarios:
Empty proxy values
Invalid proxy URLs
Mixed-case environment variable names (e.g., http_proxy, Http_Proxy)
Protocol-specific proxy routing (HTTP vs HTTPS)

Benefits

Enables seamless integration in corporate networks and environments requiring proxy configurations
Follows standard conventions by respecting common environment variables
Improves flexibility for deployment in diverse network architectures

Testing

All new functionality is covered by unit tests that verify correct behavior across different configuration scenarios.

[djmitche]

Thanks! I think this will be useful for people behind a proxy!

Unfortunately, the tests are a bit problematic: env vars are global, but Rust runs tests in parallel, so this will be flaky when the tests fight to set variables. Consider breaking the proxy creation into a separate function which takes an impl Fn(String) -> Result<String>, and testing that with closures to return the various combinations of env vars. But, also consider that these are simple conditionals and making them more complex just for testing purposes may not be worthwhile.

What do you think?

[dhufe]

Yeah, refactoring the code and put them into a separate function would be a good idea.

To address the race condition between parallel tests, wouldn't it be sufficient to declare them in this way?

use serial_test::serial;

#[test]
#[serial]
fn my_test() {
    
}

I mean, this would even be better than running all tests single threaded, or?

[djmitche]

That would help! I would suggest using a key to avoid over-serializing if we add other serialized tests in the future.

A simpler alternative would be to just do all of the tests in a single test function. Overall, I think fewer tests asserting more than just .is_ok would be good.

[dhufe]

Ok, for a actual action plan:

• refactor the proxy functionality as separate method
• make the testing either using serial_test::serial (that would be an additional dependency) or condense the test in one test method

Did I get it correctly?

[djmitche]

Sounds good!

[djmitche]

(you'll want to address the formatting and clippy too)

[dhufe]

(you'll want to address the formatting and clippy too)

Sure, I will. ;).

[djmitche]

This function is already guarded by this conditional (line 19)

[djmitche]

This makes sense in that it's possible we'll add two proxies to the client, but does mean that the testing is still "not an error". Since this function doesn't return an error, its result has no bearing on whether client() returns an error. In fact, the tests would all pass if this just returned client.

[dhufe]

Thanks for the hint, you are right.

Let me know if this addresses your feedback.

[djmitche]

Looks good! Thanks for the detailed back-and-forth!

I know these variables are typically just set on systems that require proxies, but we should still document that the sync functionality consumes and uses them. I think ServerConfig would be a good spot for that, as it affects most of those sync methods.

With that change, we can get this merged :)

[dhufe]

Ok, got it 😄 .

Please note that the final adjustments may take a few extra days due to illness.
I appreciate your patience.

[djmitche]

Awesome, thank you so much!