Skip to content

Conversation

@indutny
Copy link
Member

Not sure if the could be any other description.

cc @nodejs/crypto

@indutnyindutny added the https Issues or PRs related to the https subsystem. label Dec 12, 2015
@indutny
Copy link
MemberAuthor

@indutny
Copy link
MemberAuthor

CI is green, only unrelated failures.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a check for clientSessions.length here?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Ack.

@bnoordhuis
Copy link
Member

LGTM. Apropos the commit log: support zero maxCachedSessions or support disabling session caching?

@jasnell
Copy link
Member

LGTM. +1 to @bnoordhuis' comment regarding the commit log.

@jasnell
Copy link
Member

@nodejs/http ... I'm leaning towards semver-minor on this. thoughts?

indutny added a commit that referenced this pull request Dec 12, 2015
Zero value of `maxCachedSessions` should disable TLS session caching in `https.Agent` PR-URL: #4252 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
@indutny
Copy link
MemberAuthor

Landed in acef181, thank you!. @jasnell let's do minor just for safety.

@indutnyindutny closed this Dec 12, 2015
@indutnyindutny added the semver-minor PRs that contain new features and should be released in the next minor version. label Dec 12, 2015
@indutnyindutny deleted the feature/zero-session-cache branch December 12, 2015 17:49
indutny added a commit that referenced this pull request Dec 15, 2015
Zero value of `maxCachedSessions` should disable TLS session caching in `https.Agent` PR-URL: #4252 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
cjihrig added a commit that referenced this pull request Dec 15, 2015
Notable changes: * domains: - Fix handling of uncaught exceptions. (Julien Gilli) #3654. * https: - Added support for disabling session caching. (Fedor Indutny) #4252. * repl: - Allow third party modules to be imported using require(). This corrects a regression from 5.2.0. (Ben Noordhuis) #4215. * deps: - Upgrade libuv to 1.8.0. (Saúl Ibarra Corretgé) #4276. PR-URL: #4281
cjihrig added a commit that referenced this pull request Dec 16, 2015
Notable changes: * buffer: - Buffer.prototype.includes() has been added to keep parity with TypedArrays. (Alexander Martin) #3567. * domains: - Fix handling of uncaught exceptions. (Julien Gilli) #3654. * https: - Added support for disabling session caching. (Fedor Indutny) #4252. * repl: - Allow third party modules to be imported using require(). This corrects a regression from 5.2.0. (Ben Noordhuis) #4215. * deps: - Upgrade libuv to 1.8.0. (Saúl Ibarra Corretgé) #4276. PR-URL: #4281
cjihrig added a commit that referenced this pull request Dec 16, 2015
Notable changes: * buffer: - Buffer.prototype.includes() has been added to keep parity with TypedArrays. (Alexander Martin) #3567. * domains: - Fix handling of uncaught exceptions. (Julien Gilli) #3654. * https: - Added support for disabling session caching. (Fedor Indutny) #4252. * repl: - Allow third party modules to be imported using require(). This corrects a regression from 5.2.0. (Ben Noordhuis) #4215. * deps: - Upgrade libuv to 1.8.0. (Saúl Ibarra Corretgé) #4276. PR-URL: #4281 Conflicts: src/node_version.h
@shouze
Copy link

@indutny would you explain me why we could/should disable session reuse? I don't see any production app use cases.

@indutny
Copy link
MemberAuthor

@shouze there is one use case for the moment, and it is related to the #3940 . Basically session cache has an API problem, that is not fixed yet. So it is very practical to disable it when people want to see server certificates.

I'm going to fix #3940, but nevertheless disabling session cache should be useful for testing and, probably, other use cases.

Cheers!

scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Zero value of `maxCachedSessions` should disable TLS session caching in `https.Agent` PR-URL: nodejs#4252 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Notable changes: * buffer: - Buffer.prototype.includes() has been added to keep parity with TypedArrays. (Alexander Martin) nodejs#3567. * domains: - Fix handling of uncaught exceptions. (Julien Gilli) nodejs#3654. * https: - Added support for disabling session caching. (Fedor Indutny) nodejs#4252. * repl: - Allow third party modules to be imported using require(). This corrects a regression from 5.2.0. (Ben Noordhuis) nodejs#4215. * deps: - Upgrade libuv to 1.8.0. (Saúl Ibarra Corretgé) nodejs#4276. PR-URL: nodejs#4281 Conflicts: src/node_version.h
nicolas-moteau added a commit to Orange-OpenSource/node that referenced this pull request Mar 6, 2019
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Mar 6, 2019
PR-URL: nodejs#26433 Refs: nodejs#2228 Refs: nodejs#4252 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Mar 12, 2019
PR-URL: nodejs#26433 Refs: nodejs#2228 Refs: nodejs#4252 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
BridgeAR pushed a commit that referenced this pull request Mar 14, 2019
PR-URL: #26433 Refs: #2228 Refs: #4252 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

httpsIssues or PRs related to the https subsystem.semver-minorPRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@indutny@bnoordhuis@jasnell@shouze