Skip to content

Commit cd54e36

Browse files
committed
Forcibly clear OpenSSL's error stack
* Add a test for the bug of unwanted openSSL errors * bad_sign() would inject an error into OpenSSL's error stack * Second time of calling requestThunk would make the error be thrown * This stops stale errors from popping up later causing spurious failures * FIXME: remove me when upstream Node.js has fixed this bug
1 parent d130e33 commit cd54e36

3 files changed

Lines changed: 98 additions & 0 deletions

File tree

lib/urllib.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,18 @@ var TEXT_DATA_TYPES = [
6262
'text'
6363
];
6464

65+
// OpenSSL's error stack might already have some content stored
66+
// Forcibly clear OpenSSL's error stack before reusing an old connection.
67+
// This stops stale errors from popping up later causing spurious failures.
68+
// FIXME: remove me when upstream Node.js has fixed this bug
69+
var binding = process.binding('crypto');
70+
var NativeSecureContext = binding.SecureContext;
71+
var workaround_clear_openssl_errors = function() {
72+
var context = new NativeSecureContext();
73+
context.init();
74+
context.addRootCerts();
75+
};
76+
6577
/**
6678
* Handle all http request, both http and https support well.
6779
*
@@ -188,6 +200,7 @@ exports.requestWithCallback = function (url, args, callback) {
188200
var agent = args.agent || exports.agent;
189201

190202
if (parsedUrl.protocol === 'https:') {
203+
workaround_clear_openssl_errors();
191204
httplib = https;
192205
agent = args.httpsAgent || exports.httpsAgent;
193206
if (args.httpsAgent === false) {

test/fixtures/bad_rsa_privkey.pem

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
-----BEGIN RSA PRIVATE KEY-----
2+
MIIBUwIBADANBgkqhkiG9w0BAQEFAASCAT0wggE5AgEAAkEAz0ZHmXyxQSdWk6NF
3+
GRotTax0O94iHv843su0mOynV9QLvlAwMrUk9k4+/SwyLu0eE3iYsYgXstXi3t2u
4+
rDSIMwIDAQABAkAH4ag/Udp7m79TBdZOygwG9BPHYv7xJstGzYAkgHssf7Yd5ZuC
5+
hpKtBvWdPXZaAFbwF8NSisMl98Q/9zgB/q5BAiEA5zXuwMnwt4hE2YqzBDRFB4g9
6+
I+v+l1soy6x7Wdqo9esCIQDlf15qDb26uRDurBioE3IpZstWIIvLDdKqviZXKMs8
7+
2QIgWeC5QvA9RtsOCJLGLCg1fUwUmFYwzZ1+Kk6OVMuPSqkCIDIWFSXyL8kzoKVm
8+
O89axxyQCaqXWcsMDkEjVLzK82gpAiB7lzdDHr7MoMWwV2wC/heEFC2p0Rw4wg9j
9+
1V8QbL0Q0A==
10+
-----END RSA PRIVATE KEY-----

test/openssl.test.js

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/**!
2+
* urllib - openssl.test.js
3+
*
4+
* Copyright(c) node-modules and other contributors.
5+
* MIT Licensed
6+
*
7+
* Authors:
8+
* P.S.V.R <pmq2001@gmail.com> (http://www.ofpsvr.com)
9+
*/
10+
11+
'use strict';
12+
13+
/**
14+
* This is a test for the bug of unwanted openSSL errors.
15+
*/
16+
17+
var should = require('should');
18+
var urllib = require('../');
19+
var crypto = require('crypto');
20+
var fs = require('fs');
21+
var path = require('path');
22+
var Agent = require('agentkeepalive');
23+
var HttpsAgent = require('agentkeepalive').HttpsAgent;
24+
25+
var bad_sign = function () {
26+
// This triggers an implicit error
27+
// that is recorded onto OpenSSL's error stack
28+
var privateKey = fs.readFileSync(path.join(__dirname, './fixtures/bad_rsa_privkey.pem'), 'utf8');
29+
var s = crypto.createSign('sha1');
30+
var sign = s.sign(privateKey);
31+
};
32+
33+
describe('httpclient.test.js', function () {
34+
it('should requestThunk()', function (done) {
35+
var conf = {
36+
"keepAlive": true,
37+
"keepAliveTimeout": 300000,
38+
"timeout": 300000,
39+
"maxSockets": null,
40+
"maxFreeSockets": 10,
41+
"enableStatusLog": false
42+
};
43+
44+
var httpAgent = new Agent(conf);
45+
var httpsAgent = new HttpsAgent(conf);
46+
47+
var client = urllib.create({
48+
agent: httpAgent,
49+
httpsAgent: httpsAgent
50+
});
51+
52+
bad_sign();
53+
// 1st time when we establish a https connection,
54+
// ~ClearErrorOnReturn is called so the errors of bad_sign() won't be thrown
55+
client.requestThunk('https://iojs.org')(function (err, result) {
56+
should.not.exist(err);
57+
result.data.should.be.a.Buffer;
58+
result.status.should.equal(200);
59+
60+
setImmediate(function () {
61+
// 2nd time when we reuse an existing connection,
62+
// ~ClearErrorOnReturn will not be called,
63+
// making the errors of bad_sign() be thrown
64+
// A fix of this bug should prevent this from happening
65+
bad_sign();
66+
client.requestThunk('https://iojs.org')(function (err, result) {
67+
should.not.exist(err);
68+
result.data.should.be.a.Buffer;
69+
result.status.should.equal(200);
70+
done();
71+
});
72+
});
73+
});
74+
});
75+
});

0 commit comments

Comments
 (0)