Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

This adds an option to skip the TLS connection wrapper #1146

Closed
wants to merge 1 commit into from
Closed

This adds an option to skip the TLS connection wrapper #1146

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions nats.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,10 @@ type Options struct {
// transports.
TLSConfig *tls.Config

// SkipTLSWrapper does not upgrade the connection to TLS and is
// meant to be used if the custom dialer does handle TLS itself
SkipTLSWrapper bool

// AllowReconnect enables reconnection logic to be used when we
// encounter a disconnect from the current server.
AllowReconnect bool
Expand Down Expand Up @@ -1185,6 +1189,16 @@ func SetCustomDialer(dialer CustomDialer) Option {
}
}

// SetSkipTLSWrapper is an Option to be used with the CustomDialer which
// will not wrap the connection with TLS. Use it if the CustomDialer did
// already handle TLS
func SetSkipTLSWrapper(skip bool) Option {
return func(o *Options) error {
o.SkipTLSWrapper = skip
return nil
}
}

// UseOldRequestStyle is an Option to force usage of the old Request style.
func UseOldRequestStyle() Option {
return func(o *Options) error {
Expand Down Expand Up @@ -1894,6 +1908,10 @@ func (nc *Conn) createConn() (err error) {

// makeTLSConn will wrap an existing Conn using TLS
func (nc *Conn) makeTLSConn() error {
if nc.Opts.SkipTLSWrapper {
// we do nothing when asked to skip the TLS wrapper
return nil
}
Copy link
Member

Choose a reason for hiding this comment

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

I think we can achieve some of this with adding extra behavior to the implementation of the CustomDialer interface, to avoid having another option that is coupled to using a CustomDialer. We can have an internal interface as follows in the client, which the CustomDialer could implement:

type skipTLSDialer interface {
	SkipTLS() bool
}

// makeTLSConn will wrap an existing Conn using TLS.
func (nc *Conn) makeTLSConn() error {
	if nc.Opts.CustomDialer != nil {
		// we do nothing when asked to skip the TLS wrapper.
		sd, ok := nc.Opts.CustomDialer.(skipTLSDialer)
		if ok && sd.SkipTLS() {
			return nil
		}
	}

Then from the app perspective you would implement Dial and toggle the behavior by implementing SkipTLS like this:

type testSkipTLSDialer struct {
	dialer *net.Dialer
}

func (sd *testSkipTLSDialer) Dial(network, address string) (net.Conn, error) {
	return sd.dialer.Dial(network, address)
}

func (sd *testSkipTLSDialer) SkipTLS() bool {
	return true
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had the same though about relating that functionality to the custom dialer, but I failed to come up with a practical solution. That said, I am happy with any way to have the skip functionality merged because I do not want to deal with a fork. This way seems very unobtrusive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Who can / will decide about the way of the implementation? What can I do?

Copy link
Member

Choose a reason for hiding this comment

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

I have branch with the approach above on top of your commit, will send PR shortly for review

Copy link
Member

Choose a reason for hiding this comment

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

opened PR with this change: #1147 /cc @piotrpio

// Allow the user to configure their own tls.Config structure.
var tlsCopy *tls.Config
if nc.Opts.TLSConfig != nil {
Expand Down