From 8ceef73b8437ee97f9f96b0c50e8700096fcb843 Mon Sep 17 00:00:00 2001
From: Evan Jones <ej@evanjones.ca>
Date: Mon, 15 May 2023 11:52:26 -0400
Subject: [PATCH] pgtype.parseHstore: Reject invalid input; Fix error messages

The parseHstore function did not check the return value from
p.Consume() after a ', ' sequence. It expects a doublequote '"' that
starts the next key, but would accept any character. This means it
accepted invalid input such as:

    "key1"=>"b", ,key2"=>"value"

Add a unit test that covers this case
Fix a couple of the nearby error strings while looking at this.

Found by looking at staticcheck warnings:

    pgtype/hstore.go:434:6: this value of end is never used (SA4006)
    pgtype/hstore.go:434:6: this value of r is never used (SA4006)
---
 pgtype/hstore.go      | 13 +++++++++++--
 pgtype/hstore_test.go | 15 +++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/pgtype/hstore.go b/pgtype/hstore.go
index 5de87da9..52fe2306 100644
--- a/pgtype/hstore.go
+++ b/pgtype/hstore.go
@@ -428,12 +428,21 @@ func parseHstore(s string) (k []string, v []Text, err error) {
 				r, end = p.Consume()
 				switch {
 				case end:
-					err = errors.New("Found EOS after ',', expcting space")
+					err = errors.New("Found EOS after ',', expecting space")
 				case (unicode.IsSpace(r)):
+					// after space is a doublequote to start the key
 					r, end = p.Consume()
+					if end {
+						err = errors.New("Found EOS after space, expecting \"")
+						return
+					}
+					if r != '"' {
+						err = fmt.Errorf("Invalid character '%c' after space, expecting \"", r)
+						return
+					}
 					state = hsKey
 				default:
-					err = fmt.Errorf("Invalid character '%c' after ', ', expecting \"", r)
+					err = fmt.Errorf("Invalid character '%c' after ',', expecting space", r)
 				}
 			} else {
 				err = fmt.Errorf("Invalid character '%c' after value, expecting ','", r)
diff --git a/pgtype/hstore_test.go b/pgtype/hstore_test.go
index 5b1b5859..8ad5884a 100644
--- a/pgtype/hstore_test.go
+++ b/pgtype/hstore_test.go
@@ -230,3 +230,18 @@ func TestHstoreCodec(t *testing.T) {
 		}
 	})
 }
+
+func TestParseInvalidInputs(t *testing.T) {
+	// these inputs should be invalid, but previously were considered correct
+	invalidInputs := []string{
+		`"a"=>"1", ,b"=>"2"`,
+		`""=>"", 0"=>""`,
+	}
+	for i, input := range invalidInputs {
+		var hstore pgtype.Hstore
+		err := hstore.Scan(input)
+		if err == nil {
+			t.Errorf("test %d: input=%s (%#v) should fail; parsed correctly", i, input, input)
+		}
+	}
+}