feat: Automatically set tls.servername option with rediss:// URLs #950
Conversation
paulmelnikow
force-pushed
the
auto-tls-hostname
branch
2 times, most recently
from
9ddb978
to
90040f9
Compare
paulmelnikow
force-pushed
the
auto-tls-hostname
branch
from
90040f9
to
d6fc213
Compare
|
@luin Would you mind taking a look at this? It would be great to get this merged as Shields is currently blocked from upgrading. Thanks a lot! |
| } else if (typeof arg === "number") { | ||
| this.options.port = arg; | ||
| } else { | ||
| throw new Error("Invalid argument " + arg); | ||
| } | ||
| } | ||
| if (isTls) { | ||
| defaults(this.options, { tls: { servername: this.options.host } }); |
There was a problem hiding this comment.
I'm not sure if setting tls to this.options.host instead of true applies to more scenarios, but I think at least we need to check whether this.options.host is a hostname instead of an IP address.
As for me, I prefer the way that we just default tls options to true without setting any other properties (like servername) and let users handle it. The improvement we should make here is to keep the options provided by users not be overwritten.
There was a problem hiding this comment.
I'm not sure if setting
tlstothis.options.hostinstead oftrueapplies to more scenarios, but I think at least we need to check whetherthis.options.hostis a hostname instead of an IP address.
I'm happy to make that modification if you'd like.
Setting tls.options.host to the hostname of the server is necessary to use many shared Redis hosting services, including Compose.io and Redis Labs. It reduces boilerplate and seems like a reasonable default as this is not really a fringe use case, but a common scenario for TLS.
The improvement we should make here is to keep the options provided by users not be overwritten.
Alternatively, #949 does just that!
## [4.14.1](redis/ioredis@v4.14.0...v4.14.1) (2019-08-27) ### Bug Fixes * don’t clobber passed-in tls options with rediss:/ URLs ([#949](redis/ioredis#949)) ([ceefcfa](redis/ioredis@ceefcfa)), closes [#942](redis/ioredis#942) [#940](redis/ioredis#940) [#950](redis/ioredis#950) [#948](redis/ioredis#948)
Fix #948
Fix #940
Alternative to #949 and #942