-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
lib/hostip.c: make NAT64 address synthesis on macOS working #7121
Conversation
A working demonstration of the code: The
|
You modified two m4 files to add build flags. Those places only run if configure finds rustls or secure-transport to get used, not other TLS backends, but this name resolver thing is independent of TLS right? It seems better to have it done in a more central place for all macOS (and more?) builds, doesn't it? I assume the test build you did and proved working was done with cmake? Did you verify a configure build too? (my mac doesn't have a working IPv6 connection, I can't easily test this) |
Actually I've built it on macOS Big Sur using And yes, you're right - this is independent on the TLS backend, so it'd better to be move out. I need to figure out how to include the framework correctly for all cases (even when no/other TLS backend is used). Let's wait for the CI jobs to finish to eventually fix what the jobs find. |
The github action ones have been horribly slow the last few days though so there's a significant risk they will take 10+ hours to complete. |
I must be doing something wrong, but can't really build
|
Apparently I needed a clean build environment for |
I've moved the framework addition to a separate The I've tested builds on macOS with Thanks. |
lib/hostip.c
Outdated
@@ -529,6 +533,21 @@ enum resolve_t Curl_resolv(struct Curl_easy *data, | |||
return CURLRESOLV_ERROR; | |||
} | |||
|
|||
#ifdef ENABLE_IPV6 | |||
#if defined(USE_RESOLVE_ON_IPS) && TARGET_OS_OSX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this perhaps rather be ...&& defined(TARGET_OS_OSX)
? It risks causing a warning otherwise when not defined.
Also: where is TARGET_OS_OSX
defined? Is that from the macOS SDK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TARGET_OS_OSX
is defined in TargetConditionals.h
from the macOS SDK, yes.
I can include it explicitly in the file, currently it only seems to be included implicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more minutes of research:
TARGET_OS_OSX
is defined with value 0
for non-macOS targets (such as iOS), therefore a simple #ifdef TARGET_OS_OSX
is not the best way forward.
#if defined(TARGET_OS_OSX) && TARGET_OS_OSX == 1
on the other hand seems to do the job.
@@ -68,6 +68,10 @@ | |||
#include "curl_memory.h" | |||
#include "memdebug.h" | |||
|
|||
#ifdef USE_RESOLVE_ON_IPS | |||
#include <SystemConfiguration/SystemConfiguration.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure here if to wrap it with more conditions like the ones below - I need to check if it exists on targets like iOS. If not, the extra #if
wrapping will be needed (or another, shared #define
created elsewhere).
Thanks! |
* libcurl: Bump to 7.77.0 Signed-off-by: Alejandro Colomar <alejandro.colomar@exfo.com> * add SystemConfiguration frameworks since 7.77.0 see curl/curl#7121 * propagate frameworks for all Apple OS, not only macOS also replace Cocoa by CoreFoundation Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
* libcurl: Bump to 7.77.0 Signed-off-by: Alejandro Colomar <alejandro.colomar@exfo.com> * add SystemConfiguration frameworks since 7.77.0 see curl/curl#7121 * propagate frameworks for all Apple OS, not only macOS also replace Cocoa by CoreFoundation Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
curl#7121 introduced a macOS system call to `SCDynamicStoreCopyProxies`, which is invoked every time an IP address needs to be resolved. However, this system call is not thread-safe, and macOS will kill the process if the system call is run first in a fork. To make it possible for the parent process to call this once and prevent the crash, only invoke this system call in the global initialization routine. In addition, this change is beneficial because it: 1. Avoids extra macOS system calls for every IP lookup. 2. Consolidates macOS-specific initialization in a separate file. Closes curl#11252
#7121 introduced a macOS system call to `SCDynamicStoreCopyProxies`, which is invoked every time an IP address needs to be resolved. However, this system call is not thread-safe, and macOS will kill the process if the system call is run first in a fork. To make it possible for the parent process to call this once and prevent the crash, only invoke this system call in the global initialization routine. In addition, this change is beneficial because it: 1. Avoids extra macOS system calls for every IP lookup. 2. Consolidates macOS-specific initialization in a separate file. Fixes #11252 Closes #11254
curl#7121 introduced a macOS system call to `SCDynamicStoreCopyProxies`, which is invoked every time an IP address needs to be resolved. However, this system call is not thread-safe, and macOS will kill the process if the system call is run first in a fork. To make it possible for the parent process to call this once and prevent the crash, only invoke this system call in the global initialization routine. In addition, this change is beneficial because it: 1. Avoids extra macOS system calls for every IP lookup. 2. Consolidates macOS-specific initialization in a separate file. Fixes curl#11252 Closes curl#11254
This breaks curl on macOS 10.9 MacPorts due to failing to link SystemConfiguration Framework, causing missing SCDynamicStoreCopyProxies called from libcurl.4.dylib. The configure line checks whether we need to link macOS frameworks SystemConfiguration but for some reason answers: no. |
|
curl#7121 introduced a macOS system call to `SCDynamicStoreCopyProxies`, which is invoked every time an IP address needs to be resolved. However, this system call is not thread-safe, and macOS will kill the process if the system call is run first in a fork. To make it possible for the parent process to call this once and prevent the crash, only invoke this system call in the global initialization routine. In addition, this change is beneficial because it: 1. Avoids extra macOS system calls for every IP lookup. 2. Consolidates macOS-specific initialization in a separate file. Fixes curl#11252 Closes curl#11254
On macOS running in an IPv6-only access network where DNS64 and NAT64 functions are also deployed, the operating system is able to synthesize the NAT64 IPv6 address when an IPv4-only literal is used. This is described in
man 3 getaddrinfo
as follows:The operating system however does not perform this synthesis unless the
SCDynamicStoreCopyProxies
function from theSystemConfiguration
framework is called.This commit therefore attempts to call the function in
lib/hostip.c
ifUSE_RESOLVE_ON_IPS
is set (which is currently only set on macOS) before any hostname resolution is performed. As a result of this function call,curl
is able to perform requests such ascurl http://1.1.1.1/
even on networks where there is no IPv4 connectivity in the access network.