build: export more openssl symbols · Pull Request #7576 · nodejs/node

ghost · 2016-07-07T03:40:43Z

This makes the generated openssl.def include these additional symbols:

SSL_CTX_use_PrivateKey_file <- I use this one
SSL_CTX_use_RSAPrivateKey_file
SSL_CTX_use_certificate_chain_file <- I use this one
SSL_CTX_use_certificate_file
SSL_CTX_use_serverinfo_file
SSL_add_dir_cert_subjects_to_stack
SSL_add_file_cert_subjects_to_stack
SSL_load_client_CA_file
SSL_set_fd <- I use this one
SSL_set_rfd
SSL_set_wfd
SSL_use_PrivateKey_file
SSL_use_RSAPrivateKey_file
SSL_use_certificate_file
X509_STORE_load_locations
X509_STORE_set_default_paths
X509_load_cert_crl_file
X509_load_cert_file
X509_load_crl_file

Checklist

make -j4 test (UNIX), or vcbuild test nosign (Windows) passes

Affected core subsystem(s)

Windows build

Description of change

Export more OpenSSL symbols

mscdex · 2016-07-07T04:23:26Z

/cc @bnoordhuis

bnoordhuis · 2016-07-07T07:34:16Z

node.gyp

Can you keep the list in alphabetical order?

bnoordhuis · 2016-07-07T07:35:09Z

In principle LGTM with a style nit. The commit log should follow the project standard, though. It's described in CONTRIBUTING.md.

ghost · 2016-07-10T14:10:32Z

This is now on top of the history and I think it follows the formatting rules.

ghost · 2016-07-11T20:05:35Z

Rebased again

bnoordhuis · 2016-07-12T13:31:28Z

LGTM. CI: https://ci.nodejs.org/job/node-test-pull-request/3264/

ghost · 2016-07-12T16:15:55Z

What happens now? Should I rebase and add Reviewed By: and run the CI again?

This exports even more openssl symbols when building on Windows. SSL_set_fd is one example of added symbol. PR-URL: #7576 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

silverwind · 2016-07-12T21:52:35Z

LGTM, but I'm a bit confused why for example X509 should be available through SOCK,STDIO when a quick grep through openssl sources for OPENSSL_NO_ shows a separate X509 category:

AES,BF,BIO,BUF_FREELISTS,BUFFER,CAMELLIA,CAST,CMS,COMP,DEPRECATED,DES,DGRAM,DH,
DSA,EC,EC2M,EC_NISTP_64_GCC_128,ECDH,ECDSA,ENGINE,ERR,EVP,FP_API,GMP,GOST,
HASH_COMP,HEARTBEATS,HMAC,IDEA,JPAKE,KRB5,LHASH,MD2,MD4,MD5,MDC2,NEXTPROTONEG,
OCSP,PSK,RC2,RC4,RC5,RFC3779,RIPEMD,RSA,SCTP,SEED,SHA,SHA0,SHA1,SHA256,SHA512,
SOCK,SRP,SRTP,SSL2_METHOD,SSL3_METHOD,SSL_INTERN,SSL_TRACE,STACK,STATIC_ENGINE,
STDIO,STORE,TLSEXT,UNIT_TEST,WHIRLPOOL,X509

ghost · 2016-07-12T22:20:00Z

The STDIO category spans over multiple categories:

X509_print_ex_fp 1093 1_1_0 EXIST::FUNCTION:STDIO
PEM_write 1128 1_1_0 EXIST::FUNCTION:STDIO

I guess STDIO category holds things that modifies files in some way (SSL_set_fd is also in the STDIO category).

This exports even more openssl symbols when building on Windows. SSL_set_fd is one example of added symbol. PR-URL: #7576 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Roman Reiss <me@silverwind.io>

silverwind · 2016-07-12T22:25:38Z

Thanks for the explanation, landed in b8b8c36.

I appended "on Windows" to the commit message as from my understanding of https://github.com/nodejs/node/pull/6274/files, this gyp section only applies on Windows.

ghost · 2016-07-12T22:28:53Z

Nice 👍

ghost · 2016-07-12T22:30:06Z

Btw, will this be included in the next 6.x version or should this commit be tagged in some way for that? I would like to have it in next version.

silverwind · 2016-07-12T22:31:25Z

It will be in the next 6.x for sure (unless it gets reverted). Only semver-tagged changes would be delayed until the next minor/major.

bnoordhuis · 2016-07-13T04:35:38Z

I added the semver-minor label because it's an expansion of the API surface.

This exports even more openssl symbols when building on Windows. SSL_set_fd is one example of added symbol. PR-URL: #7576 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Roman Reiss <me@silverwind.io>

Notable changes: * build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) #7983 and #7576 * child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) #7811 and #7838 * fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) #7942 * repl: The REPL now supports editor mode. (Prince J Wesley) #7275 * util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) #8013 Refs: #8020 PR-URL: #8070

Notable changes: * build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) #7983 and #7576 * child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) #7811 and #7838 * child_process: argv[0] can now be set to arbitrary values in spawned processes. (Pat Pannuto) #7696 * fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) #7942 * repl: The REPL now supports editor mode. (Prince J Wesley) #7275 * util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) #8013 Refs: #8020 PR-URL: #8070

Notable changes: * build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) nodejs#7983 and nodejs#7576 * child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) nodejs#7811 and nodejs#7838 * child_process: argv[0] can now be set to arbitrary values in spawned processes. (Pat Pannuto) nodejs#7696 * fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) nodejs#7942 * repl: The REPL now supports editor mode. (Prince J Wesley) nodejs#7275 * util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) nodejs#8013 Refs: nodejs#8020 PR-URL: nodejs#8070

This exports even more openssl symbols when building on Windows. SSL_set_fd is one example of added symbol. PR-URL: nodejs#7576 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Roman Reiss <me@silverwind.io>

This exports even more openssl symbols when building on Windows. SSL_set_fd is one example of added symbol. PR-URL: #7576 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Roman Reiss <me@silverwind.io>

This LTS release comes with 108 commits. This includes 30 which are doc related, 28 which are test related, 16 which are build / tool related, and 4 commits which are updates to dependencies. Notable Changes: The SEMVER-MINOR changes include: * build: - export openssl symbols on Windows making it possible to build addons linked against the bundled version of openssl (Alex Hultman) #7576 * debugger: - make listen address configurable in the debugger server (Ben Noordhuis) #3316 * dgram: - generalized send queue to handle close fixing a potential throw when dgram socket is closed in the listening event handler. (Matteo Collina) #7066 * http: - Introduce the 451 status code "Unavailable For Legal Reasons" (Max Barinov) #4377 * tls: - introduce `secureContext` for `tls.connect` which is useful for caching client certificates, key, and CA certificates. (Fedor Indutny) #4246 Notable SEMVER-PATCH changes include: * build: - introduce the configure --shared option for embedders (sxa555) #6994 * gtest: - the test reporter now outputs tap comments as yamlish (Johan Bergström) #9262 * src: - node no longer aborts when c-ares initialization fails (Ben Noordhuis) #8710 * tls: - fix memory leak when writing data to TLSWrap instance during handshake (Fedor Indutny) #9586 PR-URL: #9736

This LTS release comes with 108 commits. This includes 30 which are doc related, 28 which are test related, 16 which are build / tool related, and 4 commits which are updates to dependencies. Notable Changes: The SEMVER-MINOR changes include: * build: - export openssl symbols on Windows making it possible to build addons linked against the bundled version of openssl (Alex Hultman) nodejs/node#7576 * debugger: - make listen address configurable in the debugger server (Ben Noordhuis) nodejs/node#3316 * dgram: - generalized send queue to handle close fixing a potential throw when dgram socket is closed in the listening event handler. (Matteo Collina) nodejs/node#7066 * http: - Introduce the 451 status code "Unavailable For Legal Reasons" (Max Barinov) nodejs/node#4377 * tls: - introduce `secureContext` for `tls.connect` which is useful for caching client certificates, key, and CA certificates. (Fedor Indutny) nodejs/node#4246 Notable SEMVER-PATCH changes include: * build: - introduce the configure --shared option for embedders (sxa555) nodejs/node#6994 * gtest: - the test reporter now outputs tap comments as yamlish (Johan Bergstrom) nodejs/node#9262 * src: - node no longer aborts when c-ares initialization fails (Ben Noordhuis) nodejs/node#8710 * tls: - fix memory leak when writing data to TLSWrap instance during handshake (Fedor Indutny) nodejs/node#9586 PR-URL: nodejs/node#9736 Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>

nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Jul 7, 2016

mscdex added the openssl Issues and PRs related to the OpenSSL dependency. label Jul 7, 2016

bnoordhuis reviewed Jul 7, 2016
View reviewed changes

node.gyp Outdated

Copy link

Member

bnoordhuis Jul 7, 2016

Choose a reason for hiding this comment

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

Can you keep the list in alphabetical order?

build: export more openssl symbols

d8801f6

This exports even more openssl symbols when building on Windows. SSL_set_fd is one example of added symbol. PR-URL: #7576 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

silverwind closed this Jul 12, 2016

bnoordhuis added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 13, 2016

bnoordhuis added the lts-watch-v4.x label Jul 13, 2016

cjihrig mentioned this pull request Aug 8, 2016

v6.4.0 Release Proposal #8020

Closed

cjihrig mentioned this pull request Aug 11, 2016

v6.4.0 Proposal #8070

Merged

MylesBorins added this to the v4.7.0 milestone Oct 24, 2016

MylesBorins added land-on-v4.x and removed lts-watch-v4.x labels Nov 14, 2016

MylesBorins removed this from the 4.7.0 milestone Nov 14, 2016

MylesBorins mentioned this pull request Nov 22, 2016

v4.7.0 proposal #9736

Merged

Uh oh!

Conversation

ghost commented Jul 7, 2016

Checklist

Affected core subsystem(s)

Description of change

Uh oh!

mscdex commented Jul 7, 2016

Uh oh!

bnoordhuis Jul 7, 2016

Choose a reason for hiding this comment

Uh oh!

bnoordhuis commented Jul 7, 2016

Uh oh!

ghost commented Jul 10, 2016

Uh oh!

ghost commented Jul 11, 2016

Uh oh!

bnoordhuis commented Jul 12, 2016

Uh oh!

ghost commented Jul 12, 2016

Uh oh!

silverwind commented Jul 12, 2016

Uh oh!

ghost commented Jul 12, 2016

Uh oh!

silverwind commented Jul 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Jul 12, 2016

Uh oh!

ghost commented Jul 12, 2016

Uh oh!

silverwind commented Jul 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bnoordhuis commented Jul 13, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

silverwind commented Jul 12, 2016 •

edited

Loading

silverwind commented Jul 12, 2016 •

edited

Loading