conn.go: default close handler should not return ErrCloseSent. (#865)

pnx · coreydaley · web-flow · commit 6f5d2139f4b9 · 2023-11-12T19:54:23.000-05:00
## What type of PR is this? (check all applicable) - [ ] Refactor - [ ] Feature - [x] Bug Fix - [ ] Optimization - [ ] Documentation Update - [ ] Go Version Update - [ ] Dependency Update ## Description Noticed a change in default close handler from 1.5.0 to 1.5.1 1.5.1 Now returns the error from `WriteControl` with `CloseMessage` type. However this results in ErrCloseSent returned if the connection initiated the close handshake. This is normally correct as the connection should not be able to write after a close handshake is initiated, except when calling the close handler. in that case nil should be returned so that `advanceFrame()` can return the actual close message. ## Related Tickets & Documents ## Added/updated tests? - [x] Yes - [ ] No, and this is why: _please replace this line with details on why tests have not been included_ - [ ] I need help with writing tests ## Run verifications and test - [ ] `make verify` is passing (fails with `GO-2023-2186 Incorrect detection of reserved device names on Windows in path/filepath` that is unrelated to this change) - [x] `make test` is passing Co-authored-by: Corey Daley <cdaley@redhat.com>
diff --git a/conn.go b/conn.go
@@ -1163,7 +1163,8 @@ func (c *Conn) SetCloseHandler(h func(code int, text string) error) {
 	if h == nil {
 		h = func(code int, text string) error {
 			message := FormatCloseMessage(code, "")
-			if err := c.WriteControl(CloseMessage, message, time.Now().Add(writeWait)); err != nil {
+			err := c.WriteControl(CloseMessage, message, time.Now().Add(writeWait))
+			if err != nil && err != ErrCloseSent {
 				return err
 			}
 			return nil
diff --git a/conn_test.go b/conn_test.go
@@ -477,6 +477,26 @@ func TestWriteAfterMessageWriterClose(t *testing.T) {
 	}
 }
 
+func TestWriteHandlerDoesNotReturnErrCloseSent(t *testing.T) {
+	var b1, b2 bytes.Buffer
+
+	client := newTestConn(&b2, &b1, false)
+	server := newTestConn(&b1, &b2, true)
+
+	msg := FormatCloseMessage(CloseNormalClosure, "")
+	if err := client.WriteMessage(CloseMessage, msg); err != nil {
+		t.Fatalf("unexpected error when writing close message, %v", err)
+	}
+
+	if _, _, err := server.NextReader(); !IsCloseError(err, 1000) {
+		t.Fatalf("server expects a close message, %v returned", err)
+	}
+
+	if _, _, err := client.NextReader(); !IsCloseError(err, 1000) {
+		t.Fatalf("client expects a close message, %v returned", err)
+	}
+}
+
 func TestReadLimit(t *testing.T) {
 	t.Run("Test ReadLimit is enforced", func(t *testing.T) {
 		const readLimit = 512
