From c54577a7774683cbcf45154b5af79f7a18f7c4c9 Mon Sep 17 00:00:00 2001 From: Ivan Kozlovic Date: Thu, 13 Dec 2018 11:35:34 -0800 Subject: [PATCH] [FIXED] Maintain string case in errors A permission error on a subject that would have upper case would be converted to all lower case, which is not correct. Signed-off-by: Ivan Kozlovic --- nats.go | 16 +++++++++------- nats_test.go | 30 ++++++++++++------------------ test/auth_test.go | 20 +++++++++++++------- test/cluster_test.go | 2 +- test/conn_test.go | 2 +- 5 files changed, 36 insertions(+), 34 deletions(-) diff --git a/nats.go b/nats.go index 6dfe57972..07c36f083 100644 --- a/nats.go +++ b/nats.go @@ -1529,7 +1529,7 @@ func (nc *Conn) connectProto() (string, error) { // normalizeErr removes the prefix -ERR, trim spaces and remove the quotes. func normalizeErr(line string) string { - s := strings.ToLower(strings.TrimSpace(strings.TrimPrefix(line, _ERR_OP_))) + s := strings.TrimSpace(strings.TrimPrefix(line, _ERR_OP_)) s = strings.TrimLeft(strings.TrimRight(s, "'"), "'") return s } @@ -2342,20 +2342,22 @@ func (nc *Conn) LastError() error { // processErr processes any error messages from the server and // sets the connection's lastError. -func (nc *Conn) processErr(e string) { - // Trim, remove quotes, convert to lower case. - e = normalizeErr(e) +func (nc *Conn) processErr(ie string) { + // Trim, remove quotes + ne := normalizeErr(ie) + // convert to lower case. + e := strings.ToLower(ne) // FIXME(dlc) - process Slow Consumer signals special. if e == STALE_CONNECTION { nc.processOpErr(ErrStaleConnection) } else if strings.HasPrefix(e, PERMISSIONS_ERR) { - nc.processPermissionsViolation(e) + nc.processPermissionsViolation(ne) } else if strings.HasPrefix(e, AUTHORIZATION_ERR) { - nc.processAuthorizationViolation(e) + nc.processAuthorizationViolation(ne) } else { nc.mu.Lock() - nc.err = errors.New("nats: " + e) + nc.err = errors.New("nats: " + ne) nc.mu.Unlock() nc.Close() } diff --git a/nats_test.go b/nats_test.go index 8e0fa38fe..d6a7a1d51 100644 --- a/nats_test.go +++ b/nats_test.go @@ -882,39 +882,33 @@ func TestParserSplitMsg(t *testing.T) { } func TestNormalizeError(t *testing.T) { - received := "Typical Error" - expected := strings.ToLower(received) - if s := normalizeErr("-ERR '" + received + "'"); s != expected { + expected := "Typical Error" + if s := normalizeErr("-ERR '" + expected + "'"); s != expected { t.Fatalf("Expected '%s', got '%s'", expected, s) } - received = "Trim Surrounding Spaces" - expected = strings.ToLower(received) - if s := normalizeErr("-ERR '" + received + "' "); s != expected { + expected = "Trim Surrounding Spaces" + if s := normalizeErr("-ERR '" + expected + "' "); s != expected { t.Fatalf("Expected '%s', got '%s'", expected, s) } - received = "Trim Surrounding Spaces Without Quotes" - expected = strings.ToLower(received) - if s := normalizeErr("-ERR " + received + " "); s != expected { + expected = "Trim Surrounding Spaces Without Quotes" + if s := normalizeErr("-ERR " + expected + " "); s != expected { t.Fatalf("Expected '%s', got '%s'", expected, s) } - received = "Error Without Quotes" - expected = strings.ToLower(received) - if s := normalizeErr("-ERR " + received); s != expected { + expected = "Error Without Quotes" + if s := normalizeErr("-ERR " + expected); s != expected { t.Fatalf("Expected '%s', got '%s'", expected, s) } - received = "Error With Quote Only On Left" - expected = strings.ToLower(received) - if s := normalizeErr("-ERR '" + received); s != expected { + expected = "Error With Quote Only On Left" + if s := normalizeErr("-ERR '" + expected); s != expected { t.Fatalf("Expected '%s', got '%s'", expected, s) } - received = "Error With Quote Only On Right" - expected = strings.ToLower(received) - if s := normalizeErr("-ERR " + received + "'"); s != expected { + expected = "Error With Quote Only On Right" + if s := normalizeErr("-ERR " + expected + "'"); s != expected { t.Fatalf("Expected '%s', got '%s'", expected, s) } } diff --git a/test/auth_test.go b/test/auth_test.go index dd6753477..3838304df 100644 --- a/test/auth_test.go +++ b/test/auth_test.go @@ -40,7 +40,7 @@ func TestAuth(t *testing.T) { // This test may be a bit too strict for the future, but for now makes // sure that we correctly process the -ERR content on connect. - if err.Error() != nats.ErrAuthorization.Error() { + if strings.ToLower(err.Error()) != nats.ErrAuthorization.Error() { t.Fatalf("Expected error '%v', got '%v'", nats.ErrAuthorization, err) } @@ -304,8 +304,8 @@ func TestPermViolation(t *testing.T) { Username: "ivan", Password: "pwd", Permissions: &server.Permissions{ - Publish: &server.SubjectPermission{Allow: []string{"foo"}}, - Subscribe: &server.SubjectPermission{Allow: []string{"bar"}}, + Publish: &server.SubjectPermission{Allow: []string{"Foo"}}, + Subscribe: &server.SubjectPermission{Allow: []string{"Bar"}}, }, }, } @@ -325,20 +325,26 @@ func TestPermViolation(t *testing.T) { defer nc.Close() // Cause a publish error - nc.Publish("bar", []byte("fail")) + nc.Publish("Bar", []byte("fail")) // Cause a subscribe error - nc.Subscribe("foo", func(_ *nats.Msg) {}) + nc.Subscribe("Foo", func(_ *nats.Msg) {}) expectedErrorTypes := []string{"publish", "subscription"} for _, expectedErr := range expectedErrorTypes { select { case e := <-errCh: - if !strings.Contains(e.Error(), nats.PERMISSIONS_ERR) { + if !strings.Contains(strings.ToLower(e.Error()), nats.PERMISSIONS_ERR) { t.Fatalf("Did not receive error about permissions") } - if !strings.Contains(e.Error(), expectedErr) { + if !strings.Contains(strings.ToLower(e.Error()), expectedErr) { t.Fatalf("Did not receive error about %q, got %v", expectedErr, e.Error()) } + // Make sure subject is not converted to lower case + if expectedErr == "publish" && !strings.Contains(e.Error(), "Bar") { + t.Fatalf("Subject Bar not found in error: %v", e) + } else if expectedErr == "subscribe" && !strings.Contains(e.Error(), "Foo") { + t.Fatalf("Subject Foo not found in error: %v", e) + } case <-time.After(2 * time.Second): t.Fatalf("Did not get the permission error") } diff --git a/test/cluster_test.go b/test/cluster_test.go index 13c1e1828..d67193c33 100644 --- a/test/cluster_test.go +++ b/test/cluster_test.go @@ -167,7 +167,7 @@ func TestAuthServers(t *testing.T) { t.Fatalf("Expect Auth failure, got no error\n") } - if !strings.Contains(err.Error(), "authorization") { + if !strings.Contains(err.Error(), "Authorization") { t.Fatalf("Wrong error, wanted Auth failure, got '%s'\n", err) } diff --git a/test/conn_test.go b/test/conn_test.go index bf292c6d0..a4570c9c0 100644 --- a/test/conn_test.go +++ b/test/conn_test.go @@ -1235,7 +1235,7 @@ func TestServerErrorClosesConnection(t *testing.T) { // Check LastError(), it should be "nats: " lastErr := nc.LastError().Error() - expectedErr := "nats: " + strings.ToLower(serverSentError) + expectedErr := "nats: " + serverSentError if lastErr != expectedErr { t.Fatalf("Expected error: '%v', got '%v'", expectedErr, lastErr) }