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

deps: upgrade to c-ares v1.14.0 #19939

Closed
wants to merge 3 commits into from
Closed

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Apr 11, 2018

Changelog over 1.13.0 which we're currently bundling:

  • Fix patch for CVE-2017-1000381 to not be overly aggressive.
  • win32: Preserve DNS server order returned by Windows when sorting and exclude
    DNS servers in legacy subnets.
  • win32: Support most recent Visual Studio 2017
  • gethostbyaddr should fail with ECANCELLED not ENOTFOUND when ares_cancel
    is called.
  • win32: Exclude legacy ipv6 subnets
  • android: Applications compiled for Oreo can no longer use
    __system_property_get and must use Java calls to retrieve DNS servers.
  • win32: Force use of ANSI functions
  • CMake minimum version is now 3.1
  • ares_gethostbyname.3: fix callback status values
  • docs: Document WSAStartup requirement
  • Fix a typo in init_by_resolv_conf
  • Android JNI code leaks local references in some cases
  • Force using the ANSI versions of WinAPI functions

The most important changes have already been included via

  • 50e580d deps: cherry-pick 0ef4a0c64b6 from c-ares upstream (gethostbyaddr should fail with ECANCELLED)
  • 9a0631d deps: cherry-pick 18ea996 from c-ares upstream (patch for CVE-2017-1000381)

Not much else of significance in here but there is improved Android support so the Android Node builders should be happy about that.

rvagg added 3 commits April 11, 2018 21:44
 * Fix patch for CVE-2017-1000381 to not be overly aggressive.
 * win32: Preserve DNS server order returned by Windows when sorting and exclude
   DNS servers in legacy subnets.
 * win32: Support most recent Visual Studio 2017
 * gethostbyaddr should fail with ECANCELLED not ENOTFOUND when ares_cancel
   is called.
 * win32: Exclude legacy ipv6 subnets
 * android: Applications compiled for Oreo can no longer use
   __system_property_get and must use Java calls to retrieve DNS servers.
 * win32: Force use of ANSI functions
 * CMake minimum version is now 3.1
 * ares_gethostbyname.3: fix callback status values
 * docs: Document WSAStartup requirement
 * Fix a typo in init_by_resolv_conf
 * Android JNI code leaks local references in some cases
 * Force using the ANSI versions of WinAPI functions

The most important changes have already been included via

 * 50e580d deps: cherry-pick 0ef4a0c64b6 from c-ares upstream
 * 9a0631d deps: cherry-pick 18ea996 from c-ares upstream
c-ares switched to using ares_ssize_t for platform-independent ssize_t,
our GYP usage to include config/<platform>/ares_config.h causes problems
when including gyp as a library in core, i.e. in env.h and cares_wrap.h,
where the defines don't get pulled in properly. This, so far, is the
easiest approach to just making it work nicely--explicitly defining
ares_ssize_t for the different Windows variants and ssize_t for
non-Windows where we don't have a configured type from an ares_config.h.
In all of our non-Windows platforms it is ssize_t anyway so this is
safe.

PR-URL: nodejs#15378
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Was 72c5458:

  PR-URL: nodejs#5090
  Reviewed-By: Fedor Indutny <fedor@indutny.com>

Reimplemented for c-ares 1.13.0

PR-URL: nodejs#15378
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@nodejs-github-bot nodejs-github-bot added the cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. label Apr 11, 2018
Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

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

Rubberstamp LGTM (just cross checked the original commits in c-ares with this PR).

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 11, 2018
@jasnell jasnell added this to the 10.0.0 milestone Apr 11, 2018
@rvagg rvagg requested a review from addaleax April 11, 2018 22:13
@jasnell
Copy link
Member

jasnell commented Apr 14, 2018

@BridgeAR
Copy link
Member

Landed in 4a0dff7...2b6bb9f 🎉

@BridgeAR BridgeAR closed this Apr 16, 2018
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Apr 16, 2018
 * Fix patch for CVE-2017-1000381 to not be overly aggressive.
 * win32: Preserve DNS server order returned by Windows when sorting and
   exclude DNS servers in legacy subnets.
 * win32: Support most recent Visual Studio 2017.
 * gethostbyaddr should fail with ECANCELLED not ENOTFOUND when
   ares_cancel is called.
 * win32: Exclude legacy ipv6 subnets.
 * android: Applications compiled for Oreo can no longer use
   __system_property_get and must use Java calls to retrieve DNS
   servers.
 * win32: Force use of ANSI functions.
 * CMake minimum version is now 3.1.
 * ares_gethostbyname.3: fix callback status values.
 * docs: Document WSAStartup requirement.
 * Fix a typo in init_by_resolv_conf.
 * Android JNI code leaks local references in some cases.
 * Force using the ANSI versions of WinAPI functions.

The most important changes have already been included via

 * 50e580d deps: cherry-pick 0ef4a0c64b6 from c-ares upstream
 * 9a0631d deps: cherry-pick 18ea996 from c-ares upstream

PR-URL: nodejs#19939
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Apr 16, 2018
c-ares switched to using ares_ssize_t for platform-independent ssize_t,
our GYP usage to include config/<platform>/ares_config.h causes problems
when including gyp as a library in core, i.e. in env.h and cares_wrap.h,
where the defines don't get pulled in properly. This, so far, is the
easiest approach to just making it work nicely--explicitly defining
ares_ssize_t for the different Windows variants and ssize_t for
non-Windows where we don't have a configured type from an ares_config.h.
In all of our non-Windows platforms it is ssize_t anyway so this is
safe.

PR-URL: nodejs#15378
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: nodejs#19939
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Apr 16, 2018
Was 72c5458:

  PR-URL: nodejs#5090
  Reviewed-By: Fedor Indutny <fedor@indutny.com>

Reimplemented for c-ares 1.13.0

PR-URL: nodejs#15378
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: nodejs#19939
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
jasnell pushed a commit that referenced this pull request Apr 16, 2018
 * Fix patch for CVE-2017-1000381 to not be overly aggressive.
 * win32: Preserve DNS server order returned by Windows when sorting and
   exclude DNS servers in legacy subnets.
 * win32: Support most recent Visual Studio 2017.
 * gethostbyaddr should fail with ECANCELLED not ENOTFOUND when
   ares_cancel is called.
 * win32: Exclude legacy ipv6 subnets.
 * android: Applications compiled for Oreo can no longer use
   __system_property_get and must use Java calls to retrieve DNS
   servers.
 * win32: Force use of ANSI functions.
 * CMake minimum version is now 3.1.
 * ares_gethostbyname.3: fix callback status values.
 * docs: Document WSAStartup requirement.
 * Fix a typo in init_by_resolv_conf.
 * Android JNI code leaks local references in some cases.
 * Force using the ANSI versions of WinAPI functions.

The most important changes have already been included via

 * 50e580d deps: cherry-pick 0ef4a0c64b6 from c-ares upstream
 * 9a0631d deps: cherry-pick 18ea996 from c-ares upstream

PR-URL: #19939
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
jasnell pushed a commit that referenced this pull request Apr 16, 2018
c-ares switched to using ares_ssize_t for platform-independent ssize_t,
our GYP usage to include config/<platform>/ares_config.h causes problems
when including gyp as a library in core, i.e. in env.h and cares_wrap.h,
where the defines don't get pulled in properly. This, so far, is the
easiest approach to just making it work nicely--explicitly defining
ares_ssize_t for the different Windows variants and ssize_t for
non-Windows where we don't have a configured type from an ares_config.h.
In all of our non-Windows platforms it is ssize_t anyway so this is
safe.

PR-URL: #15378
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: #19939
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
jasnell pushed a commit that referenced this pull request Apr 16, 2018
Was 72c5458:

  PR-URL: #5090
  Reviewed-By: Fedor Indutny <fedor@indutny.com>

Reimplemented for c-ares 1.13.0

PR-URL: #15378
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: #19939
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 1, 2018
 * Fix patch for CVE-2017-1000381 to not be overly aggressive.
 * win32: Preserve DNS server order returned by Windows when sorting and
   exclude DNS servers in legacy subnets.
 * win32: Support most recent Visual Studio 2017.
 * gethostbyaddr should fail with ECANCELLED not ENOTFOUND when
   ares_cancel is called.
 * win32: Exclude legacy ipv6 subnets.
 * android: Applications compiled for Oreo can no longer use
   __system_property_get and must use Java calls to retrieve DNS
   servers.
 * win32: Force use of ANSI functions.
 * CMake minimum version is now 3.1.
 * ares_gethostbyname.3: fix callback status values.
 * docs: Document WSAStartup requirement.
 * Fix a typo in init_by_resolv_conf.
 * Android JNI code leaks local references in some cases.
 * Force using the ANSI versions of WinAPI functions.

The most important changes have already been included via

 * 50e580d deps: cherry-pick 0ef4a0c64b6 from c-ares upstream
 * 9a0631d deps: cherry-pick 18ea996 from c-ares upstream

PR-URL: nodejs#19939
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 1, 2018
c-ares switched to using ares_ssize_t for platform-independent ssize_t,
our GYP usage to include config/<platform>/ares_config.h causes problems
when including gyp as a library in core, i.e. in env.h and cares_wrap.h,
where the defines don't get pulled in properly. This, so far, is the
easiest approach to just making it work nicely--explicitly defining
ares_ssize_t for the different Windows variants and ssize_t for
non-Windows where we don't have a configured type from an ares_config.h.
In all of our non-Windows platforms it is ssize_t anyway so this is
safe.

PR-URL: nodejs#15378
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: nodejs#19939
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 1, 2018
Was 72c5458:

  PR-URL: nodejs#5090
  Reviewed-By: Fedor Indutny <fedor@indutny.com>

Reimplemented for c-ares 1.13.0

PR-URL: nodejs#15378
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: nodejs#19939
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@MylesBorins
Copy link
Contributor

Should this be backported to v8.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

@MylesBorins MylesBorins self-assigned this Aug 7, 2018
@kelp404
Copy link

kelp404 commented Sep 2, 2018

Did you miss ares_android.h?
I can't buid node for android arm 64.

NDK: r17b
node: v10.9.0
  cc '-D_DARWIN_USE_64_BIT_INODE=1' '-D_LARGEFILE_SOURCE' '-D_FILE_OFFSET_BITS=64' '-D_GNU_SOURCE' '-DCARES_STATICLIB' '-DHAVE_CONFIG_H' '-D_GLIBCXX_USE_C99_MATH' -I../deps/cares/include -I../deps/cares/src -I../deps/cares/config/android  -Wall -Wextra -Wno-unused-parameter -g -pedantic -Wall -Wextra -Wno-unused-parameter -O3 -fno-omit-frame-pointer -fPIE  -MMD -MF /root/node-in-android/out/Release/.deps//root/node-in-android/out/Release/obj.target/cares/deps/cares/src/ares__close_sockets.o.d.raw  -D__ANDROID_API__=21 -c -o /root/node-in-android/out/Release/obj.target/cares/deps/cares/src/ares__close_sockets.o ../deps/cares/src/ares__close_sockets.c
In file included from ../deps/cares/src/ares_setup.h:32:0,
                 from ../deps/cares/src/ares_cancel.c:15:
../deps/cares/config/android/ares_config.h:62:1: warning: C++ style comments are not allowed in ISO C90
 //#define HAVE_ARPA_NAMESER_COMPAT_H 1
 ^
../deps/cares/config/android/ares_config.h:62:1: warning: (this will be reported only once per input file)
In file included from ../deps/cares/src/ares_setup.h:32:0,
                 from ../deps/cares/src/ares__close_sockets.c:17:
../deps/cares/config/android/ares_config.h:62:1: warning: C++ style comments are not allowed in ISO C90
 //#define HAVE_ARPA_NAMESER_COMPAT_H 1
 ^
../deps/cares/config/android/ares_config.h:62:1: warning: (this will be reported only once per input file)
In file included from ../deps/cares/src/ares_setup.h:32:0,
                 from ../deps/cares/src/ares_android.c:19:
../deps/cares/config/android/ares_config.h:62:1: warning: C++ style comments are not allowed in ISO C90
 //#define HAVE_ARPA_NAMESER_COMPAT_H 1
 ^
../deps/cares/config/android/ares_config.h:62:1: warning: (this will be reported only once per input file)
../deps/cares/src/ares_android.c:21:26: fatal error: ares_android.h: No such file or directory
 #include "ares_android.h"
                          ^
compilation terminated.
make[1]: *** [/root/node-in-android/out/Release/obj.target/cares/deps/cares/src/ares_android.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make[1]: Leaving directory `/root/node-in-android/out'
make: *** [node] Error 2
[BUILD] Build failed. See '../node-android-21-arm64/build.log'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants