Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Sources/AsyncHTTPClient/ConnectionPool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import Musl
import Android
#elseif os(Linux) || os(FreeBSD)
import Glibc
#elseif os(Windows)
import ucrt
import WinSDK
#else
#error("unsupported target operating system")
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import func Darwin.pow
import func Musl.pow
#elseif canImport(Android)
import func Android.pow
#elseif canImport(ucrt)
import func ucrt.pow
#else
import func Glibc.pow
#endif
Expand Down
22 changes: 21 additions & 1 deletion Sources/AsyncHTTPClient/HTTPClient+HTTPCookie.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ import Darwin
import Musl
#elseif canImport(Android)
import Android
#elseif os(Windows)
import ucrt
import WinSDK
#elseif canImport(Glibc)
import Glibc
#endif
Expand Down Expand Up @@ -216,12 +219,19 @@ extension String.UTF8View.SubSequence {
}
}

#if !os(Windows)
nonisolated(unsafe) private let posixLocale: UnsafeMutableRawPointer = {
// All POSIX systems must provide a "POSIX" locale, and its date/time formats are US English.
// https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap07.html#tag_07_03_05
let _posixLocale = newlocale(LC_TIME_MASK | LC_NUMERIC_MASK, "POSIX", nil)!
return UnsafeMutableRawPointer(_posixLocale)
}()
#else
nonisolated(unsafe) private let posixLocale: UnsafeMutableRawPointer = {
// FIXME: This can be cleaner. But the Windows shim doesn't need a locale pointer
UnsafeMutableRawPointer(bitPattern: 0)!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This init is documented to return nil for a bitPattern of 0 so this is a crash in disguise. If the Windows shim doesn't need a locale pointer, can we just remove this branch entirely and update the caller?

}()
#endif

private func parseTimestamp(_ utf8: String.UTF8View.SubSequence, format: String) -> tm? {
var timeComponents = tm()
Expand Down Expand Up @@ -251,6 +261,16 @@ private func parseCookieTime(_ timestampUTF8: String.UTF8View.SubSequence) -> In
else {
return nil
}
#if os(Windows)
let timegm = _mkgmtime
#endif

let timestamp = Int64(timegm(&timeComponents))
return timestamp == -1 && errno == EOVERFLOW ? nil : timestamp

#if os(Windows)
let err = GetLastError()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

_mkgmtime is a CRT function, doesn't that mean you should be checking errno? (Apologies if that's a naive Windows question...)

#else
let err = errno
#endif
return timestamp == -1 && err == EOVERFLOW ? nil : timestamp
}
203 changes: 203 additions & 0 deletions Sources/CAsyncHTTPClient/CAsyncHTTPClient.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,208 @@
#include <stdbool.h>
#include <time.h>

#if defined(_WIN32)
#include <string.h>
#include <ctype.h>
// Windows does not provide strptime/strptime_l. Implement a tiny parser that
// supports the three date formats used for cookie parsing in this package:
// 1) "%a, %d %b %Y %H:%M:%S"
// 2) "%a, %d-%b-%y %H:%M:%S"
// 3) "%a %b %d %H:%M:%S %Y"

static int month_from_abbrev(const char *p) {
// Return 0-11 for Jan..Dec, or -1 on failure.
if (!p) return -1;
switch (p[0]) {
case 'J':
if (p[1] == 'a' && p[2] == 'n') return 0; // Jan
if (p[1] == 'u' && p[2] == 'n') return 5; // Jun
if (p[1] == 'u' && p[2] == 'l') return 6; // Jul
break;
case 'F':
if (p[1] == 'e' && p[2] == 'b') return 1; // Feb
break;
case 'M':
if (p[1] == 'a' && p[2] == 'r') return 2; // Mar
if (p[1] == 'a' && p[2] == 'y') return 4; // May
break;
case 'A':
if (p[1] == 'p' && p[2] == 'r') return 3; // Apr
if (p[1] == 'u' && p[2] == 'g') return 7; // Aug
break;
case 'S':
if (p[1] == 'e' && p[2] == 'p') return 8; // Sep
break;
case 'O':
if (p[1] == 'c' && p[2] == 't') return 9; // Oct
break;
case 'N':
if (p[1] == 'o' && p[2] == 'v') return 10; // Nov
break;
case 'D':
if (p[1] == 'e' && p[2] == 'c') return 11; // Dec
break;
}
return -1;
}

static int is_wkday_abbrev(const char *p) {
// Check for valid weekday abbreviation (Mon..Sun)
// Expect exactly 3 ASCII letters.
if (!p) return 0;
char a = p[0], b = p[1], c = p[2];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We're potentially reading out of bounds here.

if (!isalpha((unsigned char)a) || !isalpha((unsigned char)b) || !isalpha((unsigned char)c)) return 0;
// Accept common English abbreviations, case-sensitive as typically emitted.
return (a=='M'&&b=='o'&&c=='n')||(a=='T'&&b=='u'&&c=='e')||(a=='W'&&b=='e'&&c=='d')||
(a=='T'&&b=='h'&&c=='u')||(a=='F'&&b=='r'&&c=='i')||(a=='S'&&b=='a'&&c=='t')||
(a=='S'&&b=='u'&&c=='n');
Comment on lines +76 to +78

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a little hard to parse without any whitespace. It'd probably benefit from having one day one per line.

}

static int parse_1to2_digits(const char **pp) {
const char *p = *pp;
if (!isdigit((unsigned char)p[0])) return -1;
int val = p[0]-'0';
p++;
if (isdigit((unsigned char)p[0])) {
val = val*10 + (p[0]-'0');
p++;
}
*pp = p;
return val;
}

static int parse_fixed2(const char **pp) {
const char *p = *pp;
if (!isdigit((unsigned char)p[0]) || !isdigit((unsigned char)p[1])) return -1;
int val = (p[0]-'0')*10 + (p[1]-'0');
p += 2;
*pp = p;
return val;
}

static int parse_fixed4(const char **pp) {
const char *p = *pp;
for (int i = 0; i < 4; i++) {
if (!isdigit((unsigned char)p[i])) return -1;
}
int val = (p[0]-'0')*1000 + (p[1]-'0')*100 + (p[2]-'0')*10 + (p[3]-'0');
p += 4;
*pp = p;
return val;
}

static int expect_char(const char **pp, char c) {
if (**pp != c) return 0;
(*pp)++;
return 1;
}

static int expect_space(const char **pp) {
if (**pp != ' ') return 0;
(*pp)++;
return 1;
}

static int parse_time_hms(const char **pp, int *h, int *m, int *s) {
int hh = parse_fixed2(pp); if (hh < 0) return 0;
if (!expect_char(pp, ':')) return 0;
int mm = parse_fixed2(pp); if (mm < 0) return 0;
if (!expect_char(pp, ':')) return 0;
int ss = parse_fixed2(pp); if (ss < 0) return 0;
if (hh > 23 || mm > 59 || ss > 60) return 0; // allow leap second 60
*h = hh; *m = mm; *s = ss;
Comment on lines +127 to +133

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you avoid multiple statements per line? It generally just makes readability worse.

return 1;
}

static void init_tm_utc(struct tm *out) {
memset(out, 0, sizeof(*out));
out->tm_isdst = 0;
}

static bool parse_cookie_format1(const char *p, struct tm *out) {
// "%a, %d %b %Y %H:%M:%S"
if (!is_wkday_abbrev(p)) return false;
p += 3;
if (!expect_char(&p, ',')) return false;
if (!expect_space(&p)) return false;
int mday = parse_1to2_digits(&p); if (mday < 1 || mday > 31) return false;
if (!expect_space(&p)) return false;
int mon = month_from_abbrev(p); if (mon < 0) return false; p += 3;
if (!expect_space(&p)) return false;
int year = parse_fixed4(&p); if (year < 1601) return false;
if (!expect_space(&p)) return false;
int hh, mm, ss; if (!parse_time_hms(&p, &hh, &mm, &ss)) return false;
if (*p != '\0') return false;
init_tm_utc(out);
out->tm_mday = mday;
out->tm_mon = mon;
out->tm_year = year - 1900;
out->tm_hour = hh; out->tm_min = mm; out->tm_sec = ss;
return true;
}

static bool parse_cookie_format2(const char *p, struct tm *out) {
// "%a, %d-%b-%y %H:%M:%S"
if (!is_wkday_abbrev(p)) return false;
p += 3;
if (!expect_char(&p, ',')) return false;
if (!expect_space(&p)) return false;
int mday = parse_1to2_digits(&p); if (mday < 1 || mday > 31) return false;
if (!expect_char(&p, '-')) return false;
int mon = month_from_abbrev(p); if (mon < 0) return false; p += 3;
if (!expect_char(&p, '-')) return false;
int y2 = parse_fixed2(&p); if (y2 < 0) return false;
int year = (y2 >= 70) ? (1900 + y2) : (2000 + y2);
if (!expect_space(&p)) return false;
int hh, mm, ss; if (!parse_time_hms(&p, &hh, &mm, &ss)) return false;
if (*p != '\0') return false;
init_tm_utc(out);
out->tm_mday = mday;
out->tm_mon = mon;
out->tm_year = year - 1900;
out->tm_hour = hh; out->tm_min = mm; out->tm_sec = ss;
return true;
}

static bool parse_cookie_format3(const char *p, struct tm *out) {
// "%a %b %d %H:%M:%S %Y"
if (!is_wkday_abbrev(p)) return false;
p += 3;
if (!expect_space(&p)) return false;
int mon = month_from_abbrev(p); if (mon < 0) return false; p += 3;
if (!expect_space(&p)) return false;
int mday = parse_1to2_digits(&p); if (mday < 1 || mday > 31) return false;
if (!expect_space(&p)) return false;
int hh, mm, ss; if (!parse_time_hms(&p, &hh, &mm, &ss)) return false;
if (!expect_space(&p)) return false;
int year = parse_fixed4(&p); if (year < 1601) return false;
if (*p != '\0') return false;
init_tm_utc(out);
out->tm_mday = mday;
out->tm_mon = mon;
out->tm_year = year - 1900;
out->tm_hour = hh; out->tm_min = mm; out->tm_sec = ss;
return true;
}

static bool parse_cookie_timestamp_windows(const char *string, const char *format, struct tm *result) {
(void)format; // format ignored: we try the three known patterns regardless.
return parse_cookie_format1(string, result) ||
parse_cookie_format2(string, result) ||
parse_cookie_format3(string, result);
}

bool swiftahc_cshims_strptime(const char * string, const char * format, struct tm * result) {
return parse_cookie_timestamp_windows(string, format, result);
}

bool swiftahc_cshims_strptime_l(const char * string, const char * format, struct tm * result, void * locale) {
(void)locale; // locale is ignored on Windows; we always use POSIX month/weekday names.
return parse_cookie_timestamp_windows(string, format, result);
}
#endif // _WIN32

#if !defined(_WIN32)
bool swiftahc_cshims_strptime(const char * string, const char * format, struct tm * result) {
const char * firstNonProcessed = strptime(string, format, result);
if (firstNonProcessed) {
Expand All @@ -41,3 +243,4 @@ bool swiftahc_cshims_strptime_l(const char * string, const char * format, struct
}
return false;
}
#endif // _WIN32
12 changes: 8 additions & 4 deletions Tests/AsyncHTTPClientTests/HTTP2ClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -426,13 +426,17 @@ class HTTP2ClientTests: XCTestCase {
XCTAssertNoThrow(
maybeServer = try ServerBootstrap(group: serverGroup)
.serverChannelOption(ChannelOptions.socketOption(.so_reuseaddr), value: 1)
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEPORT), value: 1)
#if !os(Windows)
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEPORT), value: 1)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

formatting looks off here

#endif
.childChannelInitializer { channel in
channel.close()
}
.childChannelOption(ChannelOptions.socketOption(.so_reuseaddr), value: 1)
.bind(host: "127.0.0.1", port: serverPort)
.wait()
#if !os(Windows)
.childChannelOption(ChannelOptions.socketOption(.so_reuseaddr), value: 1)
#endif
.bind(host: "127.0.0.1", port: serverPort)
.wait()
)
// shutting down the old server closes all connections immediately
XCTAssertNoThrow(try bin.shutdown())
Expand Down
2 changes: 2 additions & 0 deletions Tests/AsyncHTTPClientTests/HTTPClientCookieTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,7 @@ class HTTPClientCookieTests: XCTestCase {
XCTAssertEqual("abc\"", c?.value)
}

#if !os(Windows)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you move this inside the text and do an XCTSkip for Windows? That way we can document why the test is skipped.

func testCookieExpiresDateParsingWithNonEnglishLocale() throws {
try withCLocaleSetToGerman {
// Check that we are using a German C locale.
Expand All @@ -500,4 +501,5 @@ class HTTPClientCookieTests: XCTestCase {
XCTAssertNil(c?.expires)
}
}
#endif
}
26 changes: 20 additions & 6 deletions Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ import Musl
import Android
#elseif canImport(Glibc)
import Glibc
#elseif os(Windows)
import WinSDK
#endif

/// Are we testing NIO Transport services
Expand All @@ -74,14 +76,17 @@ let canBindIPv6Loopback: Bool = {
let elg = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer { try! elg.syncShutdownGracefully() }
let serverChannel = try? ServerBootstrap(group: elg)
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
#if !os(Windows)
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
#endif
.bind(host: "::1", port: 0)
.wait()
let didBind = (serverChannel != nil)
try! serverChannel?.close().wait()
return didBind
}()

#if !os(Windows)
/// Runs the given block in the context of a non-English C locale (in this case, German).
/// Throws an XCTSkip error if the locale is not supported by the system.
func withCLocaleSetToGerman(_ body: () throws -> Void) throws {
Expand All @@ -99,6 +104,7 @@ func withCLocaleSetToGerman(_ body: () throws -> Void) throws {
defer { _ = uselocale(oldLocale) }
try body()
}
#endif

final class TestHTTPDelegate: HTTPClientResponseDelegate {
typealias Response = Void
Expand Down Expand Up @@ -263,7 +269,13 @@ enum TemporaryFileHelpers {
let templateBytesCount = templateBytes.count
let fd = templateBytes.withUnsafeMutableBufferPointer { ptr in
ptr.baseAddress!.withMemoryRebound(to: Int8.self, capacity: templateBytesCount) { ptr in
#if os(Windows)
// _mktemp_s is not great, as it's rumored to have limited randomness, but Windows doesn't have mkstemp
// And this is a test utility only.
Comment on lines +273 to +274

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There's no rumor about this:

_mktemp_s can create a maximum of 26 unique file names for any given combination of base and nameTemplate values.

From the docs: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/mktemp-s-wmktemp-s?view=msvc-170

This is such a limitation that I wonder if tests might legitimately hit this.

_mktemp_s(ptr, templateBytesCount)
#else
mkstemp(ptr)
#endif
}
}
templateBytes.removeLast()
Expand Down Expand Up @@ -516,11 +528,13 @@ where
let connectionIDAtomic = ManagedAtomic(0)

let serverChannel = try! ServerBootstrap(group: self.group)
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
.serverChannelOption(
ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEPORT),
value: reusePort ? 1 : 0
)
#if !os(Windows)
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
.serverChannelOption(
ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEPORT),
value: reusePort ? 1 : 0
)
#endif
.serverChannelInitializer { [activeConnCounterHandler] channel in
channel.pipeline.addHandler(activeConnCounterHandler)
}.childChannelInitializer { channel in
Expand Down
Loading
Loading