Skip to content

Commit f753253

Browse files
committed
http: ajust header callback according to review
1 parent 37d44e4 commit f753253

File tree

1 file changed

+103
-104
lines changed

1 file changed

+103
-104
lines changed

src/http/Client.zig

Lines changed: 103 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -582,9 +582,6 @@ pub const Transfer = struct {
582582
// connection in makeRequest().
583583
_use_proxy: bool = undefined,
584584

585-
// stateful variables used to parse responses headers.
586-
_resp_header_status: enum { empty, first, next, end } = .empty,
587-
588585
fn deinit(self: *Transfer) void {
589586
self.req.headers.deinit();
590587
if (self._handle) |handle| {
@@ -695,122 +692,124 @@ pub const Transfer = struct {
695692

696693
const header = buffer[0 .. buf_len - 2];
697694

698-
// transition the status dependending the previous one.
699-
transfer._resp_header_status = switch (transfer._resp_header_status) {
700-
.empty => .first,
701-
.first => .next,
702-
.next => .next,
703-
.end => .first,
704-
};
705-
706-
// mark the end of parsing headers
707-
if (buf_len == 2) transfer._resp_header_status = .end;
708-
709-
switch (transfer._resp_header_status) {
710-
.empty => unreachable,
711-
.first => {
712-
if (buf_len < 13) {
713-
log.debug(.http, "invalid response line", .{ .line = header });
695+
if (transfer.response_header == null) {
696+
if (transfer._redirecting and buf_len == 2) {
697+
// parse and set cookies for the redirection.
698+
redirectionCookies(transfer, easy) catch |err| {
699+
log.debug(.http, "redirection cookies", .{ .err = err });
714700
return 0;
701+
};
702+
return buf_len;
703+
}
704+
705+
if (buf_len < 13 or std.mem.startsWith(u8, header, "HTTP/") == false) {
706+
if (transfer._redirecting) {
707+
return buf_len;
715708
}
716-
const version_start: usize = if (header[5] == '2') 7 else 9;
717-
const version_end = version_start + 3;
709+
log.debug(.http, "invalid response line", .{ .line = header });
710+
return 0;
711+
}
712+
const version_start: usize = if (header[5] == '2') 7 else 9;
713+
const version_end = version_start + 3;
718714

719-
// a bit silly, but it makes sure that we don't change the length check
720-
// above in a way that could break this.
721-
std.debug.assert(version_end < 13);
715+
// a bit silly, but it makes sure that we don't change the length check
716+
// above in a way that could break this.
717+
std.debug.assert(version_end < 13);
722718

723-
const status = std.fmt.parseInt(u16, header[version_start..version_end], 10) catch {
724-
log.debug(.http, "invalid status code", .{ .line = header });
725-
return 0;
726-
};
719+
const status = std.fmt.parseInt(u16, header[version_start..version_end], 10) catch {
720+
log.debug(.http, "invalid status code", .{ .line = header });
721+
return 0;
722+
};
727723

728-
var url: [*c]u8 = undefined;
729-
errorCheck(c.curl_easy_getinfo(easy, c.CURLINFO_EFFECTIVE_URL, &url)) catch |err| {
730-
log.err(.http, "failed to get URL", .{ .err = err });
731-
return 0;
732-
};
724+
if (status >= 300 and status <= 399) {
725+
transfer._redirecting = true;
726+
return buf_len;
727+
}
728+
transfer._redirecting = false;
733729

734-
// When using proxy, curl call the header function for all HTTP
735-
// requests, including the CONNECT one used tunneling requests.
736-
if (transfer._use_proxy and transfer.proxy_response_header == null) {
737-
transfer.proxy_response_header = .{
738-
.url = "",
739-
.status = status,
740-
};
741-
742-
// We want to ignore the successful proxy's CONNECT request.
743-
// But there is no proper way to detect if the current
744-
// request is a proxy CONNECT one.
745-
// We know curl uses a CONNECT when it establishes a TLS
746-
// conn.
747-
if (status == 200 and std.mem.startsWith(u8, std.mem.span(url), "https")) {
748-
return buf_len;
749-
}
750-
}
730+
var url: [*c]u8 = undefined;
731+
errorCheck(c.curl_easy_getinfo(easy, c.CURLINFO_EFFECTIVE_URL, &url)) catch |err| {
732+
log.err(.http, "failed to get URL", .{ .err = err });
733+
return 0;
734+
};
751735

752-
if (status >= 300 and status <= 399) {
753-
transfer._redirecting = true;
754-
return buf_len;
755-
}
756-
transfer._redirecting = false;
736+
transfer.response_header = .{
737+
.url = url,
738+
.status = status,
739+
};
740+
transfer.bytes_received = buf_len;
741+
return buf_len;
742+
}
757743

758-
transfer.response_header = .{
759-
.url = url,
760-
.status = status,
761-
};
762-
},
763-
.next => {},
764-
.end => {
765-
// If we are in a redirection, take care of cookies only.
766-
if (transfer._redirecting) {
767-
// parse and set cookies for the redirection.
768-
redirectionCookies(transfer, easy) catch |err| {
769-
log.debug(.http, "redirection cookies", .{ .err = err });
770-
return 0;
771-
};
772-
return buf_len;
773-
}
744+
transfer.bytes_received += buf_len;
774745

775-
if (transfer._use_proxy and transfer.response_header == null) {
776-
// we are in a successful CONNECT proxy request, ignore it.
777-
return buf_len;
778-
}
746+
if (buf_len != 2) {
747+
return buf_len;
748+
}
779749

780-
if (getResponseHeader(easy, "content-type", 0)) |ct| {
781-
var hdr = &transfer.response_header.?;
782-
const value = ct.value;
783-
const len = @min(value.len, ResponseHeader.MAX_CONTENT_TYPE_LEN);
784-
hdr._content_type_len = len;
785-
@memcpy(hdr._content_type[0..len], value[0..len]);
786-
}
750+
// Starting here, we get the last header line.
751+
752+
// We're connecting to a proxy. Consider the first request to the
753+
// proxy's result.
754+
if (transfer._use_proxy and transfer.proxy_response_header == null) {
755+
// We have to cases:
756+
// 1. for http://, we have one request. So both
757+
// proxy_response_header and response_header will have the same
758+
// value.
759+
//
760+
// 2. for https://, we two successive requests, a CONNECT to the
761+
// proxy and a final request. So proxy_response_header and
762+
// response_header may have different values.
763+
transfer.proxy_response_header = transfer.response_header;
764+
765+
// Detect if the request is a CONNECT to the proxy. There might be
766+
// a better way to detect this, but I didn't find a better one.
767+
// When we don't force curl to always use tunneling, it uses
768+
// CONNECT tunnel only for https requests.
769+
const is_connect = std.mem.startsWith(u8, std.mem.span(transfer.proxy_response_header.?.url), "https");
770+
771+
// If the CONNECT is successful, curl will create a following
772+
// request to the final target, so we reset
773+
// transfer.response_header to get the "real" data.
774+
if (is_connect and transfer.proxy_response_header.?.status == 200) {
775+
transfer.response_header = null;
776+
return buf_len;
777+
}
787778

788-
var i: usize = 0;
789-
while (true) {
790-
const ct = getResponseHeader(easy, "set-cookie", i);
791-
if (ct == null) break;
792-
transfer.req.cookie_jar.populateFromResponse(&transfer.uri, ct.?.value) catch |err| {
793-
log.err(.http, "set cookie", .{ .err = err, .req = transfer });
794-
};
795-
i += 1;
796-
if (i >= ct.?.amount) break;
797-
}
779+
// If the CONNECT fails, use the request result as it would be our
780+
// final request.
781+
}
798782

799-
transfer.req.header_callback(transfer) catch |err| {
800-
log.err(.http, "header_callback", .{ .err = err, .req = transfer });
801-
// returning < buf_len terminates the request
802-
return 0;
803-
};
783+
if (getResponseHeader(easy, "content-type", 0)) |ct| {
784+
var hdr = &transfer.response_header.?;
785+
const value = ct.value;
786+
const len = @min(value.len, ResponseHeader.MAX_CONTENT_TYPE_LEN);
787+
hdr._content_type_len = len;
788+
@memcpy(hdr._content_type[0..len], value[0..len]);
789+
}
804790

805-
if (transfer.client.notification) |notification| {
806-
notification.dispatch(.http_response_header_done, &.{
807-
.transfer = transfer,
808-
});
809-
}
810-
},
791+
var i: usize = 0;
792+
while (true) {
793+
const ct = getResponseHeader(easy, "set-cookie", i);
794+
if (ct == null) break;
795+
transfer.req.cookie_jar.populateFromResponse(&transfer.uri, ct.?.value) catch |err| {
796+
log.err(.http, "set cookie", .{ .err = err, .req = transfer });
797+
};
798+
i += 1;
799+
if (i >= ct.?.amount) break;
811800
}
812801

813-
transfer.bytes_received += buf_len;
802+
transfer.req.header_callback(transfer) catch |err| {
803+
log.err(.http, "header_callback", .{ .err = err, .req = transfer });
804+
// returning < buf_len terminates the request
805+
return 0;
806+
};
807+
808+
if (transfer.client.notification) |notification| {
809+
notification.dispatch(.http_response_header_done, &.{
810+
.transfer = transfer,
811+
});
812+
}
814813
return buf_len;
815814
}
816815

0 commit comments

Comments
 (0)