v0.10.33 does not build when openssl disabled

[lovebug356]

v0.10.33 does not build when openssl disabled, v0.10.32 builds fine if openssl is disabled.

In file included from ../src/node.cc:85:0:
../src/node_crypto.h:30:25: fatal error: openssl/ssl.h: No such file or directory

include <openssl/ssl.h>

[joerg-krause]

We have the same problem at Buildroot.

It is a bug in src/node.cc introduced by commit d601c76 which includes node_crypto.h twice. Once with HAVE_OPENSSL condition and once without:

#if HAVE_OPENSSL
# include "node_crypto.h"
#endif
[...]
#include "node_crypto.h"

So you're doomed to build with openssl...

[jsiei97]

This seems to be fixed on master, but a quick fix for v0.10.33 is to ifdef the SSL2_ENABLE with HAVE_OPENSSL.

in src/node.cc at line 2582

#if HAVE_OPENSSL
    } else if (strcmp(arg, "--enable-ssl2") == 0) {
      SSL2_ENABLE = true;
      argv[i] = const_cast<char*>("");
    } else if (strcmp(arg, "--enable-ssl3") == 0) {
      SSL3_ENABLE = true;
      argv[i] = const_cast<char*>("");
#endif

[jsiei97]

Cant reproduce on cfcb1de so this issue can probably be closed.

[joerg-krause]

We are using the same patch at Buidlroot: http://git.buildroot.net/buildroot/tree/package/nodejs/nodejs-0004-fix-build-error-without-OpenSSL-support.patch

It's not fixed at master since must work is done in the 0.12 branch. The commit cfcb1de is from 2 Oct.

The SSL2 and SSL3 handling are fixes done in this way only on the stable branch 0.10.xx.

[jsiei97]

Sorry, missed the different branches...

[joerg-krause]

This issue is still not fixed in v0.10.34 :-(

[tedu]

Still an error with v0.12.0. Same fix.

[misterdjules]

@lovebug356 @joerg-krause @jsiei97 @tedu Thank you for your help, and sorry for the delay. Adding this issue to the v0.10.40 release milestone, because we want to get v0.10.39 out as quickly as possible due to OpenSSL vulnerabilities and we don't want to add anything more to that milestone.

[misterdjules]

I'm available to guide anyone willing to pick this issue up and submit a PR that fixes it. Ping me on GitHub by mentioning @misterdjules or find me on IRC (jgi in #libuv on Freenode).

EDIT: I'm also available by email at jgilli at fastmail dot fm.

[whitlockjc]

I'd like to take a shot at this. I'll get started and will hit you up with any questions I come up with.

[misterdjules]

@whitlockjc Excellent! Thank you 👍

[whitlockjc]

I can reproduce with the latest v0.10 branch using ./configure --without-ssl && make:

Undefined symbols for architecture x86_64:
  "node::SSL2_ENABLE", referenced from:
      node::Init(int, char**) in node.o
  "node::SSL3_ENABLE", referenced from:
      node::Init(int, char**) in node.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I'll get a PR together shortly.

[joerg-krause]

This is the patch we use to build nodejs 0.10.40 with Buildroot: http://git.buildroot.net/buildroot/tree/package/nodejs/0.10.40/0004-fix-build-error-without-OpenSSL-support.patch

And this is the PR I already did to fix this issue: #8761

[whitlockjc]

Cool, I didn't see your work before. I'll stop working on this then. I got pulled into other work yesterday so after reproducing, I didn't work on the fix.

[whitlockjc]

@joerg-krause Your PR in #8761 seems to reference a deleted repository and cannot be reopened. Do you want to close that PR and create a new one, linking to this issue? If not, I don't mind creating a new PR to fix this issue I just don't want to do it without giving you the chance to.

[joerg-krause]

@whitlockjc Done so in PR #25862.

So this can actually be closed, but I don't see a Comment and Close button here 😕

[joerg-krause]

Sorry, I know why I cannot close this! I am not the opener 😄

[whitlockjc]

I don't think you want to close this. :) This will be closed when your PR is resubmitted and accepted.

[jasnell]

Yeah, just go ahead and leave this open for now. We'll close when the corrected PR lands.

[jasnell]

Landed.