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

std.http: more proxy support, buffer writes, tls toggle #17407

Merged
merged 12 commits into from
Oct 22, 2023

Conversation

truemedian
Copy link
Contributor

@truemedian truemedian commented Oct 5, 2023

std.http.Client Changes

Proxies are now separated, so HTTP and HTTPS requests can be diverted thru different proxies. Connection pool now accepts a Allocator instead of a *Client for allocations. Now passes around a *Connection instead of a *ConnectionPool.Node.

Writes are now buffered, and reads now use iovecs to reduce syscalls.

Adds client.loadDefaultProxies to loads proxy information from common
environment variables (http_proxy, HTTP_PROXY, https_proxy, HTTPS_PROXY,
all_proxy, ALL_PROXY).

  • no_proxy and NO_PROXY are currently unsupported.

Other Associated Fixes

std.Uri has expanded formatting capabilities so to enable more control over what appears in the output.

std.crypto.tls.Client.readv was evidently not tested, it was missing a parameter.

std_options Changes

Removed http_connection_pool_size, Added http_disable_tls

  • http_connection_pool_size: replaced with client.connection_pool.resize(client.allocator, size)
  • http_disable_tls: When true, all HTTPS requests will error with error.TlsInitializationFailed, and std.http will make no references to std.crypto.tls (which may reduce the size of the overall binary, and potentially reduce compile times; unless referenced elsewhere).

When compiling without TLS: test/standalone/http.zig compiles 10 seconds faster, and is half the size (aprox. -4MB in ReleaseSafe).

Documentation

Adds documentation to public functions that was missing documentation. Adds information about when order-specific functions need to be called.

Revisited old documentation and pulled it in line with what each function is actually doing now.

@truemedian truemedian force-pushed the http-ng branch 5 times, most recently from 9a9fda6 to 6a430a4 Compare October 8, 2023 01:29
@binarycraft007
Copy link
Contributor

binarycraft007 commented Oct 8, 2023

hi @truemedian, thanks for working on http proxy, does this support socks5? I didn't see any socks5 protocol handling. related: #15048.

@truemedian
Copy link
Contributor Author

does this support socks5?

No SOCKS support yet, just HTTP proxying (when you send a fully qualified uri to a proxy server and it makes a request for you) and HTTP tunneling (when you make a CONNECT request to a proxy to open a tunnel to the target).

@truemedian truemedian force-pushed the http-ng branch 6 times, most recently from aef5515 to b28f425 Compare October 14, 2023 15:39
@truemedian truemedian force-pushed the http-ng branch 2 times, most recently from 5f9d682 to f0e981c Compare October 18, 2023 02:19
@andrewrk
Copy link
Member

How's it going? You looking for a merge yet?

@truemedian
Copy link
Contributor Author

I think so. I don't have anything else important to fix or add in this batch.

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I only have superficial changes requested, but one of them regards the public API, so I do think it should be changed before merging. After this however I think it is ready to land 👍

lib/std/http/Client.zig Outdated Show resolved Hide resolved
lib/std/http/Client.zig Show resolved Hide resolved
lib/std/http/Client.zig Outdated Show resolved Hide resolved
lib/std/http/Client.zig Outdated Show resolved Hide resolved
lib/std/http/Client.zig Outdated Show resolved Hide resolved
lib/std/http/Client.zig Outdated Show resolved Hide resolved
lib/std/http/Client.zig Outdated Show resolved Hide resolved
lib/std/http/Client.zig Outdated Show resolved Hide resolved
lib/std/http/Client.zig Show resolved Hide resolved
@andrewrk
Copy link
Member

I forgot to mention, I think this PR solves #17422.

@truemedian
Copy link
Contributor Author

This at least mostly solves #17422 for std.http.Client, but I don't want to close that yet because std.http.Server likely still suffers this problem.

@andrewrk
Copy link
Member

andrewrk commented Oct 22, 2023

Looks ready to land to me, I think it only needs a zig fmt in order to pass the CI.

adds connectTunnel to form a HTTP CONNECT tunnel to the desired host.
Primarily implemented for proxies, but like connectUnix may be called by
any user.

adds loadDefaultProxies to load proxy information from common
environment variables (http_proxy, HTTP_PROXY, https_proxy, HTTPS_PROXY,
all_proxy, ALL_PROXY).
- no_proxy and NO_PROXY are currently unsupported.

splits proxy into http_proxy and https_proxy, adds headers field for
arbitrary headers to each proxy.
std_options.http_connection_pool_size removed in favor of

```
client.connection_pool.resize(client.allocator, size);
```

std_options.http_disable_tls will remove all https capability from
std.http when true. Any https request will error with
`error.TlsInitializationFailed`.

Solves ziglang#17051.
Response.do was renamed to Response.start to mimic the
naming scheme in http.Client
@andrewrk andrewrk enabled auto-merge October 22, 2023 06:42
@andrewrk andrewrk merged commit b82459f into ziglang:master Oct 22, 2023
@ncnnnnn
Copy link

ncnnnnn commented Oct 26, 2023

I am a newcomer to Zig and encountered a network link error when trying to compile zsl using Zig

Under Chinese WIN10, loadDefaultProxies cannot use system settings correctly

ZIG: 0.12.0-dev.1270+6c9d34bce
win: WIN10 21H2

The test code is as follows:

const std = @import("std");
const os = std.os;
const parseInt = std.fmt.parseInt;
const mem = std.mem;
const http = std.http;
const Server = http.Server;
const Client = http.Client;

const max_header_size = 8192;

var gpa_server = std.heap.GeneralPurposeAllocator(.{ .stack_trace_frames = 12 }){};
var gpa_client = std.heap.GeneralPurposeAllocator(.{ .stack_trace_frames = 12 }){};

const salloc = gpa_server.allocator();
const calloc = gpa_client.allocator();


test "parse integers" { 
	const key_w = comptime std.unicode.utf8ToUtf16LeStringLiteral("http_proxy");
	const key_slice = mem.sliceTo(key_w, 0);

	 const ptr = os.windows.peb().ProcessParameters.Environment;
	     var i: usize = 0;
    while (ptr[i] != 0) {
        const key_start = i;

        // There are some special environment variables that start with =,
        // so we need a special case to not treat = as a key/value separator
        // if it's the first character.
        // https://devblogs.microsoft.com/oldnewthing/20100506-00/?p=14133
        if (ptr[key_start] == '=') i += 1;

        while (ptr[i] != 0 and ptr[i] != '=') : (i += 1) {}
        const this_key = ptr[key_start..i];

        if (ptr[i] == '=') i += 1;

        const value_start = i;
        while (ptr[i] != 0) : (i += 1) {}
        const this_value = ptr[value_start..i :0];

       if (os.windows.eqlIgnoreCaseWTF16(key_slice, this_key)) {
	   std.debug.print(
            "{u} has a field called {u} \n",
            .{
                this_key,this_value,
            },
			);
           // return ;
        }
		
        i += 1; // skip over null byte
    }
	
	    var client = Client{ .allocator = calloc };
    errdefer client.deinit();
    // defer client.deinit(); handled below

    try client.loadDefaultProxies();
	std.debug.print(
            "https_proxy:: {any}  \n http_proxy:: {any} \n",
            .{
                client.https_proxy,client.http_proxy,
            },
			);
	 var h = http.Headers{ .allocator = calloc };
        defer h.deinit();
		 const uri = try std.Uri.parse("http://baidu.com");
	        var req = try client.open(.GET,uri , h, .{});
        defer req.deinit();

        try req.send(.{});
        try req.wait();	
		const body = try req.reader().readAllAlloc(calloc, 8192);
        defer calloc.free(body);
		 std.debug.print(
            "{s} has a field called {u} \n",
            .{
                body,body,
            },
			);
}

No proxy can be used to access http/https under cmd:

 { H, T, T, P, _, P, R, O, X, Y } has a field called { 1, ,,, }
https_proxy:: null
 http_proxy:: null

powershell Ability to use HTTP_ PROXY access to http, cannot use https_ Proxy access to https:

 { H, T, T, P, S, _, P, R, O, X, Y } has a field called { h, t, t, p, :, /, /, 1, ,,,}
https_proxy:: null
 http_proxy:: http.Client.Proxy{

I hope to receive your help and support thanks.

@truemedian
Copy link
Contributor Author

#17732 should fix both of these.

SuperAuguste pushed a commit to zigtools/zls that referenced this pull request Nov 22, 2023
zig project (ziglang/zig#17407)
add client.loadDefaultProxies to loads proxy environment variables
(http_proxy, HTTP_PROXY, https_proxy, HTTPS_PROXY, all_proxy, ALL_PROXY).
@truemedian truemedian deleted the http-ng branch September 6, 2024 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants