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.

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

all: add framework lib/websocket #12

Merged
merged 1 commit into from
Jun 5, 2023
Merged

all: add framework lib/websocket #12

merged 1 commit into from
Jun 5, 2023

Conversation

shuLhan
Copy link
Contributor

@shuLhan shuLhan commented Jun 4, 2023

The lib/websocket [1] is one of the Go WebSocket library that use Linux epoll or BSD kqueue.

[1] https://pkg.go.dev/github.com/shuLhan/share/lib/websocket

The lib/websocket [1] is one of the Go WebSocket library that use
Linux epoll or BSD kqueue.

[1] https://pkg.go.dev/github.com/shuLhan/share/lib/websocket
@lesismal lesismal merged commit 8761ae9 into lesismal:main Jun 5, 2023
@lesismal
Copy link
Owner

lesismal commented Jun 5, 2023

Thanks for PR!

I just read the code of lib/websocket, and found that it should have the same problem as gobwas+poller, because it doesn't implement async parser, but still provide Blocking reading-related interface{}, if some connections a slow, it will block here in the for-loop, and cause other connections waiting for long:
https://github.com/shuLhan/share/blob/master/lib/websocket/server.go#L266

So, I think it's not suitable to use it in production.

For more details:
gobwas/ws#143

Sorry to say that I will revert this PR.

@shuLhan
Copy link
Contributor Author

shuLhan commented Jun 5, 2023 via email

@lesismal
Copy link
Owner

lesismal commented Jun 5, 2023

Will you accept the PR again once I found a way to fix it?

Yes, of course!
PR will always be welcomed!

@shuLhan
Copy link
Contributor Author

shuLhan commented Jun 5, 2023

@lesismal

After reading several of your comments on gobwas/ws issues 143, gobwas/ws-examples issues 18, and then re-read my code, and test with your code at gobwas/ws-examples/issues/18#issue-867317941, I can say that the lib/websocket does not have blocking issue.

Epool in lib/websocket set the socket to nonblock for each accepted socket [1].
If a client send partial HTTP upgrade, for example one byte at atime, server will reject it immediately [2].
This is by design on HTTP level.

For example, given slow client (SC) and server (S), assume that SC try to send upgrade request "GET / HTTP/1.1" but send only one byte "G"

SC> G
S< G
S> HTTP/1.1 400 Bad Request\r\n...
S> CLOSE

In case client connection has been upgraded and client send partial websocket frame its already handled here [3].
Since the socket is non-block all partial packet received by server will be read and stored until one frame is completed.

So, I think it's not suitable to use it in production.

It's already battle tested in production.

If you can show the code that the lib/websocket has blocking issue, I would be happy to fix it.

[1] https://github.com/shuLhan/share/blob/2d57327dbe2d2d2d819d699f543daac9a55e48e9/lib/net/poll_linux.go#L47
[2] https://github.com/shuLhan/share/blob/2d57327dbe2d2d2d819d699f543daac9a55e48e9/lib/websocket/handshake.go#L149
[3] https://github.com/shuLhan/share/blob/2d57327dbe2d2d2d819d699f543daac9a55e48e9/lib/websocket/frame.go#L330

@lesismal
Copy link
Owner

lesismal commented Jun 5, 2023

@shuLhan lib/websocket's problem is even worse than gobwas/ws, it's not the blocking problem, but easy to failed when parsing streaming data.

package main

import (
	"log"
	"net"
	"time"

	"github.com/shuLhan/share/lib/websocket"
)

var (
	srv *websocket.Server

	addr      = "127.0.0.1:8080"
	handshake = []byte("GET /ws HTTP/1.1\r\n" +
		"Host: 127.0.0.1:8080\r\n" +
		"User-Agent: Go-http-client/1.1\r\n" +
		"Connection: Upgrade\r\n" +
		"Sec-Websocket-Key: w9CBg2aFf0pZFILulHE1Ww==\r\n" +
		"Sec-Websocket-Version: 13\r\n" +
		"Upgrade: websocket\r\n\r\n")
)

func handleText(conn int, payload []byte) {
	packet := websocket.NewFrameText(false, payload)

	err := websocket.Send(conn, packet)
	if err != nil {
		log.Println("handleText: " + err.Error())
	}
	log.Println("onMessage: ", string(payload))
}

var chWait = make(chan int)

func main() {
	time.AfterFunc(time.Second, func() {
		client(0)
		client(time.Second / 100)
	})

	opts := &websocket.ServerOptions{
		Address: addr,
		// HandleAuth:  handleAuth,
		ConnectPath: `/ws`,
		HandleText:  handleText,
	}
	srv := websocket.NewServer(opts)
	srv.Start()
}

func client(sleepTime time.Duration) bool {
	conn, err := net.Dial("tcp", addr)
	if err != nil {
		log.Fatalf("dial failed: %v", err)
	}

	// normal client, send full packet and if lucky the full packet arrive at the same unix.Read
	if sleepTime == 0 {
		_, err := conn.Write(handshake)
		if err != nil {
			log.Fatalf("write handshake failed: %v", err)
		}

	} else {
		// slow client, seperate the full packet to multi packets;
		// you will get err when conn.Read below.
		// in public internet, it's normal that the tcp-streaming data comes not in the same time,
		// or maybe just not in the same unix.Read, that's so easy to result to an error when your
		// parser assume the full-packet data comes  1 by 1 and everytime your unix.Read get a full packet exactly.
		_, err := conn.Write(handshake[:5])
		if err != nil {
			log.Fatalf("write half handshake failed: %v", err)
		}
		close(chWait)
		time.Sleep(sleepTime)

		_, err = conn.Write(handshake[5:])
		if err != nil {
			log.Fatalf("write half handshake failed: %v", err)
		}
	}

	buf := make([]byte, 2048)
	n, err := conn.Read(buf)
	if err != nil {
		log.Fatalf("read handshake failed: %v", err)
	}
	log.Printf("response: \n%v", string(buf[:n]))

	return n > 0
}

output:

root@ubuntu:~/slow# go run test.go 
2023/06/05 17:34:14 response: 
HTTP/1.1 101 Switching Protocols
Upgrade: websocket
Connection: Upgrade
Sec-Websocket-Accept: 29EdDSe9X5uqHkhTQj6jglN2hok=

2023/06/05 17:34:14 response: 
HTTP/1.1 400 invalid HTTP pragma: : bad request  ## look at here, when unix.Read doesn't get the full packet, just make it failed then the connection can not go on

@shuLhan
Copy link
Contributor Author

shuLhan commented Jun 5, 2023

// in public internet, it's normal that the tcp-streaming data comes not in the same time,
// or maybe just not in the same unix.Read, that's so easy to result to an error when your
// parser assume the full-packet data comes 1 by 1 and everytime your unix.Read get a full packet exactly

Yes it is normal, but this part should already handled in OS level, by buffering packet before sending or receiving.

HTTP/1.1 400 invalid HTTP pragma: : bad request ## look at here, when unix.Read doesn't get the full packet, just make it failed then the connection can not go on

Yes it is by design.
A minimum WebSocket handshake is 128 bytes.
A TCP payload in single packet can handle at least 1460 bytes.
If the client cannot send full handshake in single write then it is not worth to pursue, otherwise the server will open to slow attack, a.k.a slowloris.

I think you are mixing HTTP and WebSocket.
Unlike HTTP, WebSocket is full-duplex, both server and client try to keep the connection persistent as long as possible.
Maintaining clients that have persistent, stable connections is preferred over handling client that cannot even send full handshake.

@lesismal
Copy link
Owner

lesismal commented Jun 6, 2023

Yes it is by design.
A minimum WebSocket handshake is 128 bytes.
A TCP payload in single packet can handle at least 1460 bytes.

the TCP protocol can't promise the full handshake arrives in 1 Read.

@lesismal
Copy link
Owner

lesismal commented Jun 6, 2023

And, see another example, your socket fd is not nonblocking, so after Accept, if the socket doesn't send even 1 byte, your Recv here will block:
https://github.com/shuLhan/share/blob/master/lib/websocket/server.go#L266

package main

import (
	"log"
	"net"
	"net/url"
	"time"

	gorillaWS "github.com/gorilla/websocket"
	"github.com/shuLhan/share/lib/websocket"
)

var (
	srv *websocket.Server

	addr      = "127.0.0.1:8080"
	handshake = []byte("GET /ws HTTP/1.1\r\n" +
		"Host: 127.0.0.1:8080\r\n" +
		"User-Agent: Go-http-client/1.1\r\n" +
		"Connection: Upgrade\r\n" +
		"Sec-Websocket-Key: w9CBg2aFf0pZFILulHE1Ww==\r\n" +
		"Sec-Websocket-Version: 13\r\n" +
		"Upgrade: websocket\r\n\r\n")
)

func handleText(conn int, payload []byte) {
	packet := websocket.NewFrameText(false, payload)

	err := websocket.Send(conn, packet)
	if err != nil {
		log.Println("handleText: " + err.Error())
	}
	log.Println("onMessage: ", string(payload))
}

var chWaitDial = make(chan int)

func main() {
	time.AfterFunc(time.Second, func() {
		go client(time.Second * 30)

		// step 2: waiting for the first client dial success
		//         then start to dial with timeout
		<-chWaitDial
		u := url.URL{Scheme: "ws", Host: addr, Path: "/ws"}
		gorillaWS.DefaultDialer.HandshakeTimeout = time.Second
		conn, _, err := gorillaWS.DefaultDialer.Dial(u.String(), nil)
		if err != nil {
			// will get error of timeout here
			log.Fatalf("dial failed: %v", err)
		}
		log.Printf("gorillaWS dial: %v", err)
		conn.WriteMessage(gorillaWS.TextMessage, []byte("hello"))
	})

	opts := &websocket.ServerOptions{
		Address:     addr,
		ConnectPath: `/ws`,
		HandleText:  handleText,
	}
	srv := websocket.NewServer(opts)
	srv.Start()
}

func client(sleepTime time.Duration) bool {
	conn, err := net.Dial("tcp", addr)
	if err != nil {
		log.Fatalf("dial failed: %v", err)
	}
	// step 1: after dialing success, and before sending handshake, this client sleep for a while
	//         but allow the normal client to dial
	close(chWaitDial)

	if sleepTime > 0 {
		time.Sleep(sleepTime)
	}

	_, err = conn.Write(handshake)
	if err != nil {
		log.Fatalf("write handshake failed: %v", err)
	}

	buf := make([]byte, 2048)
	n, err := conn.Read(buf)
	if err != nil {
		log.Fatalf("read handshake failed: %v", err)
	}
	log.Printf("response: \n%v", string(buf[:n]))

	return n > 0
}

output:

root@ubuntu:~/slow# go run ./test.go 
2023/06/06 01:37:07 dial failed: read tcp 127.0.0.1:33196->127.0.0.1:8080: i/o timeout
exit status 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants