From 2743089b54b0165511e474eb2b9083a45d76b2c2 Mon Sep 17 00:00:00 2001
From: Agniva De Sarker <agnivade@yahoo.co.in>
Date: Thu, 18 Mar 2021 22:27:53 +0530
Subject: [PATCH] MM-34000: Use non-epoll mode for TLS connections (#17172)

* MM-34000: Use non-epoll mode for TLS connections

A *crypto/tls.Conn does not expose the underlying TCP connection
or even a File method to get the underlying file descriptor
like the way a *net/TCPConn does. Therefore the netpoll code would
fail to get the file descriptor.

Relevant issue here: https://github.com/mailru/easygo/issues/3

It is indeed possible to use reflect black magic to get the unexported
member, but I have found unexpected errors during writing to the websocket
by getting the file descriptor this way. I do not want to spend time investigating
this especially since this is already released.

Once this is out, we can decide on the right way to fix this, most probably
by proposing to expose the File method or some other way.

https://mattermost.atlassian.net/browse/MM-34000

```release-note
Fix an issue where websockets wouldn't work with TLS connections.
In that case, we just fall back to the way it works for Windows machines,
which is to use a separate goroutine for reader connection.
```

* Ignore logging errors on non-epoll

On non-epoll systems, we needed to return an error
to break from the loop. But in that case, there is no
need to log the error
---
 api4/websocket.go |  3 +--
 app/web_conn.go   | 28 ++++++++++++++++++----------
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/api4/websocket.go b/api4/websocket.go
index 7c12bccf8b..b2e3387802 100644
--- a/api4/websocket.go
+++ b/api4/websocket.go
@@ -5,7 +5,6 @@ package api4
 
 import (
 	"net/http"
-	"runtime"
 	"time"
 
 	"github.com/gobwas/ws"
@@ -49,7 +48,7 @@ func connectWebSocket(c *Context, w http.ResponseWriter, r *http.Request) {
 		c.App.HubRegister(wc)
 	}
 
-	if runtime.GOOS == "windows" {
+	if !wc.Epoll() {
 		wc.BlockingPump()
 	} else {
 		go wc.Pump()
diff --git a/app/web_conn.go b/app/web_conn.go
index 607ffd74fb..5235d0f940 100644
--- a/app/web_conn.go
+++ b/app/web_conn.go
@@ -36,6 +36,8 @@ const (
 	webConnMemberCacheTime = 1000 * 60 * 30 // 30 minutes
 )
 
+var errNonEpollConnClose = errors.New("connection closed")
+
 // WebConn represents a single websocket connection to a user.
 // It contains all the necessary state to manage sending/receiving data to/from
 // a websocket.
@@ -55,7 +57,7 @@ type WebConn struct {
 	send                      chan model.WebSocketMessage
 	sessionToken              atomic.Value
 	session                   atomic.Value
-	isWindows                 bool
+	hasEpoll                  bool
 	endWritePump              chan struct{}
 	pumpFinished              chan struct{}
 	closeOnce                 sync.Once
@@ -78,7 +80,7 @@ func (a *App) NewWebConn(ws net.Conn, session model.Session, t i18n.TranslateFun
 		UserId:             session.UserId,
 		T:                  t,
 		Locale:             locale,
-		isWindows:          runtime.GOOS == "windows",
+		hasEpoll:           *a.Config().ServiceSettings.ConnectionSecurity == "" && runtime.GOOS != "windows",
 		endWritePump:       make(chan struct{}),
 		pumpFinished:       make(chan struct{}),
 	}
@@ -87,8 +89,7 @@ func (a *App) NewWebConn(ws net.Conn, session model.Session, t i18n.TranslateFun
 	wc.SetSessionToken(session.Token)
 	wc.SetSessionExpiresAt(session.ExpiresAt)
 
-	// epoll/kqueue is not available on Windows.
-	if !wc.isWindows {
+	if wc.hasEpoll {
 		wc.startPoller()
 	}
 	return wc
@@ -101,7 +102,7 @@ func (a *App) NewWebConn(ws net.Conn, session model.Session, t i18n.TranslateFun
 func (wc *WebConn) Close() {
 	wc.closeOnce.Do(func() {
 		wc.WebSocket.Close()
-		if !wc.isWindows {
+		if wc.hasEpoll {
 			// This triggers the pump exit.
 			// If the pump has already exited, this just becomes a noop.
 			close(wc.endWritePump)
@@ -145,6 +146,11 @@ func (wc *WebConn) SetSession(v *model.Session) {
 	wc.session.Store(v)
 }
 
+// Epoll returns whether the websocket is eligible to use epoll or not.
+func (wc *WebConn) Epoll() bool {
+	return wc.hasEpoll
+}
+
 // Pump starts the WebConn instance. After this, the websocket
 // is ready to send messages.
 // This is only used by *nix platforms.
@@ -156,7 +162,7 @@ func (wc *WebConn) Pump() {
 	close(wc.pumpFinished)
 }
 
-// BlockingPump is the Windows alternative of Pump.
+// BlockingPump is the non-epoll alternative of Pump.
 // It creates two goroutines - one for reading, another
 // for writing.
 func (wc *WebConn) BlockingPump() {
@@ -242,9 +248,9 @@ func (wc *WebConn) ReadMsg() error {
 	switch hdr.OpCode {
 	case ws.OpClose:
 		// Return if closed.
-		// We need to return an error for Windows to let the reader exit.
-		if wc.isWindows {
-			return errors.New("connection closed")
+		// We need to return an error for non-epoll systems to let the reader exit.
+		if !wc.hasEpoll {
+			return errNonEpollConnClose
 		}
 		return nil
 	case ws.OpPong:
@@ -274,7 +280,9 @@ func (wc *WebConn) readPump() {
 
 	for {
 		if err := wc.ReadMsg(); err != nil {
-			wc.logSocketErr("websocket.read", err)
+			if err != errNonEpollConnClose {
+				wc.logSocketErr("websocket.read", err)
+			}
 			return
 		}
 	}