From c9292c44e604ff434b39af08d0a250c877fc00f6 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Sat, 12 Nov 2016 11:42:07 -0500 Subject: [PATCH 01/21] Adds aclitem[] len 1 ability --- values.go | 18 ++++++++++++++++++ values_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/values.go b/values.go index 6cb6e429..2f64f2d5 100644 --- a/values.go +++ b/values.go @@ -45,6 +45,7 @@ const ( Float4ArrayOid = 1021 Float8ArrayOid = 1022 AclItemOid = 1033 + AclItemArrayOid = 1034 InetArrayOid = 1041 VarcharOid = 1043 DateOid = 1082 @@ -77,6 +78,7 @@ var DefaultTypeFormats map[string]int16 func init() { DefaultTypeFormats = map[string]int16{ + "_aclitem": TextFormatCode, // Pg's src/backend/utils/adt/acl.c has only in/out (text) not send/recv (bin) "_bool": BinaryFormatCode, "_bytea": BinaryFormatCode, "_cidr": BinaryFormatCode, @@ -981,6 +983,8 @@ func Encode(wbuf *WriteBuf, oid Oid, arg interface{}) error { return Encode(wbuf, oid, v) case string: return encodeString(wbuf, oid, arg) + case []AclItem: + return encodeAclItemSlice(wbuf, oid, arg) case []byte: return encodeByteSlice(wbuf, oid, arg) case [][]byte: @@ -1224,6 +1228,8 @@ func Decode(vr *ValueReader, d interface{}) error { *v = decodeFloat4(vr) case *float64: *v = decodeFloat8(vr) + case *[]AclItem: + *v = decodeAclItemArray(vr) case *[]bool: *v = decodeBoolArray(vr) case *[]int16: @@ -2993,6 +2999,18 @@ func decodeTextArray(vr *ValueReader) []string { return a } +// XXX: encodeAclItemSlice; using text encoding, not binary +func encodeAclItemSlice(w *WriteBuf, oid Oid, value []AclItem) error { + w.WriteInt32(int32(len("{=r/postgres}"))) + w.WriteBytes([]byte("{=r/postgres}")) + return nil +} + +// XXX: decodeAclItemArray; using text encoding, not binary +func decodeAclItemArray(vr *ValueReader) []AclItem { + return []AclItem{"=r/postgres"} +} + func encodeStringSlice(w *WriteBuf, oid Oid, slice []string) error { var elOid Oid switch oid { diff --git a/values_test.go b/values_test.go index 8b85ceef..c2a89d79 100644 --- a/values_test.go +++ b/values_test.go @@ -643,6 +643,42 @@ func TestNullX(t *testing.T) { } } +func TestAclArrayDecoding(t *testing.T) { + t.Parallel() + + conn := mustConnect(t, *defaultConnConfig) + defer closeConn(t, conn) + tests := []struct { + sql string + query interface{} + scan interface{} + assert func(*testing.T, interface{}, interface{}) + }{ + { + "select $1::aclitem[]", + []pgx.AclItem{"=r/postgres"}, + &[]pgx.AclItem{}, + func(t *testing.T, query, scan interface{}) { + if !reflect.DeepEqual(query, *(scan.(*[]pgx.AclItem))) { + t.Errorf("failed to encode aclitem[]") + } + }, + }, + } + for i, tt := range tests { + err := conn.QueryRow(tt.sql, tt.query).Scan(tt.scan) + if err != nil { + t.Errorf(`%d. error reading array: %v`, i, err) + if pgerr, ok := err.(pgx.PgError); ok { + t.Errorf(`%d. error reading array (detail): %s`, i, pgerr.Detail) + } + continue + } + tt.assert(t, tt.query, tt.scan) + ensureConnValid(t, conn) + } +} + func TestArrayDecoding(t *testing.T) { t.Parallel() From a80ef6d35fbe82480d58b1278b8fe55af5bd12e0 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Sat, 12 Nov 2016 11:46:07 -0500 Subject: [PATCH 02/21] Actually takes the first arg --- values.go | 5 +++-- values_test.go | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/values.go b/values.go index 2f64f2d5..0723189c 100644 --- a/values.go +++ b/values.go @@ -3001,8 +3001,9 @@ func decodeTextArray(vr *ValueReader) []string { // XXX: encodeAclItemSlice; using text encoding, not binary func encodeAclItemSlice(w *WriteBuf, oid Oid, value []AclItem) error { - w.WriteInt32(int32(len("{=r/postgres}"))) - w.WriteBytes([]byte("{=r/postgres}")) + str := "{" + value[0] + "}" + w.WriteInt32(int32(len(str))) + w.WriteBytes([]byte(str)) return nil } diff --git a/values_test.go b/values_test.go index c2a89d79..92ec01a6 100644 --- a/values_test.go +++ b/values_test.go @@ -643,6 +643,7 @@ func TestNullX(t *testing.T) { } } +// XXX func TestAclArrayDecoding(t *testing.T) { t.Parallel() From 36bdbd7cb105417f883270555f853b2cc17c43b5 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Sat, 12 Nov 2016 11:56:04 -0500 Subject: [PATCH 03/21] Parses actual return string ...but only handles aclitem[] size 1 --- values.go | 11 ++++++++++- values_test.go | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/values.go b/values.go index 0723189c..251671bc 100644 --- a/values.go +++ b/values.go @@ -3009,7 +3009,16 @@ func encodeAclItemSlice(w *WriteBuf, oid Oid, value []AclItem) error { // XXX: decodeAclItemArray; using text encoding, not binary func decodeAclItemArray(vr *ValueReader) []AclItem { - return []AclItem{"=r/postgres"} + if vr.Len() == -1 { + vr.Fatal(ProtocolError("Cannot decode null into []AclItem")) + return nil + } + + str := vr.ReadString(vr.Len()) + // remove the '{' at the front and the '}' at the end + str = str[1 : len(str)-1] + return []AclItem{AclItem(str)} + // return []AclItem{"=r/postgres"} } func encodeStringSlice(w *WriteBuf, oid Oid, slice []string) error { diff --git a/values_test.go b/values_test.go index 92ec01a6..326335e0 100644 --- a/values_test.go +++ b/values_test.go @@ -661,7 +661,7 @@ func TestAclArrayDecoding(t *testing.T) { &[]pgx.AclItem{}, func(t *testing.T, query, scan interface{}) { if !reflect.DeepEqual(query, *(scan.(*[]pgx.AclItem))) { - t.Errorf("failed to encode aclitem[]") + t.Errorf("failed to encode aclitem[]\n EXPECTED: %v\n ACTUAL: %v", query, *(scan.(*[]pgx.AclItem))) } }, }, From 7d7bc873964b2dedd64a24a5769e92e2381d6847 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Sat, 12 Nov 2016 12:01:03 -0500 Subject: [PATCH 04/21] Moves sql outside of struct --- values.go | 1 - values_test.go | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/values.go b/values.go index 251671bc..e0d56351 100644 --- a/values.go +++ b/values.go @@ -3018,7 +3018,6 @@ func decodeAclItemArray(vr *ValueReader) []AclItem { // remove the '{' at the front and the '}' at the end str = str[1 : len(str)-1] return []AclItem{AclItem(str)} - // return []AclItem{"=r/postgres"} } func encodeStringSlice(w *WriteBuf, oid Oid, slice []string) error { diff --git a/values_test.go b/values_test.go index 326335e0..4c62007c 100644 --- a/values_test.go +++ b/values_test.go @@ -649,14 +649,14 @@ func TestAclArrayDecoding(t *testing.T) { conn := mustConnect(t, *defaultConnConfig) defer closeConn(t, conn) + + sql := "select $1::aclitem[]" tests := []struct { - sql string query interface{} scan interface{} assert func(*testing.T, interface{}, interface{}) }{ { - "select $1::aclitem[]", []pgx.AclItem{"=r/postgres"}, &[]pgx.AclItem{}, func(t *testing.T, query, scan interface{}) { @@ -667,7 +667,7 @@ func TestAclArrayDecoding(t *testing.T) { }, } for i, tt := range tests { - err := conn.QueryRow(tt.sql, tt.query).Scan(tt.scan) + err := conn.QueryRow(sql, tt.query).Scan(tt.scan) if err != nil { t.Errorf(`%d. error reading array: %v`, i, err) if pgerr, ok := err.(pgx.PgError); ok { From d9ab2197539c064100e1ae4a7e0ca8dc36156217 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Sat, 12 Nov 2016 12:07:48 -0500 Subject: [PATCH 05/21] Pulls out aclitem[] assert func --- values_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/values_test.go b/values_test.go index 4c62007c..46652d4d 100644 --- a/values_test.go +++ b/values_test.go @@ -643,6 +643,12 @@ func TestNullX(t *testing.T) { } } +func assertAclItemSlicesEqual(t *testing.T, query, scan interface{}) { + if !reflect.DeepEqual(query, *(scan.(*[]pgx.AclItem))) { + t.Errorf("failed to encode aclitem[]\n EXPECTED: %v\n ACTUAL: %v", query, *(scan.(*[]pgx.AclItem))) + } +} + // XXX func TestAclArrayDecoding(t *testing.T) { t.Parallel() @@ -652,18 +658,12 @@ func TestAclArrayDecoding(t *testing.T) { sql := "select $1::aclitem[]" tests := []struct { - query interface{} - scan interface{} - assert func(*testing.T, interface{}, interface{}) + query interface{} + scan interface{} }{ { []pgx.AclItem{"=r/postgres"}, &[]pgx.AclItem{}, - func(t *testing.T, query, scan interface{}) { - if !reflect.DeepEqual(query, *(scan.(*[]pgx.AclItem))) { - t.Errorf("failed to encode aclitem[]\n EXPECTED: %v\n ACTUAL: %v", query, *(scan.(*[]pgx.AclItem))) - } - }, }, } for i, tt := range tests { @@ -675,7 +675,7 @@ func TestAclArrayDecoding(t *testing.T) { } continue } - tt.assert(t, tt.query, tt.scan) + assertAclItemSlicesEqual(t, tt.query, tt.scan) ensureConnValid(t, conn) } } From 104c01df218e7a68cb25e5a87ddbd08942f3082d Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Sat, 12 Nov 2016 12:28:31 -0500 Subject: [PATCH 06/21] Handles aclitem lists of 1+ --- values.go | 20 +++++++++++++++++--- values_test.go | 4 ++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/values.go b/values.go index e0d56351..395b59ce 100644 --- a/values.go +++ b/values.go @@ -3000,8 +3000,15 @@ func decodeTextArray(vr *ValueReader) []string { } // XXX: encodeAclItemSlice; using text encoding, not binary -func encodeAclItemSlice(w *WriteBuf, oid Oid, value []AclItem) error { - str := "{" + value[0] + "}" +func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { + // cast aclitems into strings so we can use strings.Join + strs := make([]string, len(aclitems)) + for i := range strs { + strs[i] = string(aclitems[i]) + } + + str := strings.Join(strs, ",") + str = "{" + str + "}" w.WriteInt32(int32(len(str))) w.WriteBytes([]byte(str)) return nil @@ -3017,7 +3024,14 @@ func decodeAclItemArray(vr *ValueReader) []AclItem { str := vr.ReadString(vr.Len()) // remove the '{' at the front and the '}' at the end str = str[1 : len(str)-1] - return []AclItem{AclItem(str)} + strs := strings.Split(str, ",") + + // cast strings into AclItems before returning + aclitems := make([]AclItem, len(strs)) + for i := range aclitems { + aclitems[i] = AclItem(strs[i]) + } + return aclitems } func encodeStringSlice(w *WriteBuf, oid Oid, slice []string) error { diff --git a/values_test.go b/values_test.go index 46652d4d..83244b1b 100644 --- a/values_test.go +++ b/values_test.go @@ -665,6 +665,10 @@ func TestAclArrayDecoding(t *testing.T) { []pgx.AclItem{"=r/postgres"}, &[]pgx.AclItem{}, }, + { + []pgx.AclItem{"=r/postgres", "postgres=arwdDxt/postgres"}, + &[]pgx.AclItem{}, + }, } for i, tt := range tests { err := conn.QueryRow(sql, tt.query).Scan(tt.scan) From 96b652cc95e53d52315dcb33e5a3ea69bc13f19e Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Sat, 12 Nov 2016 12:36:55 -0500 Subject: [PATCH 07/21] Makes aclitem test types more specific --- values_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/values_test.go b/values_test.go index 83244b1b..662980c2 100644 --- a/values_test.go +++ b/values_test.go @@ -643,9 +643,9 @@ func TestNullX(t *testing.T) { } } -func assertAclItemSlicesEqual(t *testing.T, query, scan interface{}) { - if !reflect.DeepEqual(query, *(scan.(*[]pgx.AclItem))) { - t.Errorf("failed to encode aclitem[]\n EXPECTED: %v\n ACTUAL: %v", query, *(scan.(*[]pgx.AclItem))) +func assertAclItemSlicesEqual(t *testing.T, query, scan []pgx.AclItem) { + if !reflect.DeepEqual(query, scan) { + t.Errorf("failed to encode aclitem[]\n EXPECTED: %v\n ACTUAL: %v", query, scan) } } @@ -658,20 +658,20 @@ func TestAclArrayDecoding(t *testing.T) { sql := "select $1::aclitem[]" tests := []struct { - query interface{} - scan interface{} + query []pgx.AclItem + scan []pgx.AclItem }{ { []pgx.AclItem{"=r/postgres"}, - &[]pgx.AclItem{}, + []pgx.AclItem{}, }, { []pgx.AclItem{"=r/postgres", "postgres=arwdDxt/postgres"}, - &[]pgx.AclItem{}, + []pgx.AclItem{}, }, } for i, tt := range tests { - err := conn.QueryRow(sql, tt.query).Scan(tt.scan) + err := conn.QueryRow(sql, tt.query).Scan(&tt.scan) if err != nil { t.Errorf(`%d. error reading array: %v`, i, err) if pgerr, ok := err.(pgx.PgError); ok { From b12a1bb8bccd00071a22e94af41fc0dc11ff6525 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Sat, 12 Nov 2016 12:38:30 -0500 Subject: [PATCH 08/21] Removes scan from test struct --- values_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/values_test.go b/values_test.go index 662980c2..17a98197 100644 --- a/values_test.go +++ b/values_test.go @@ -657,21 +657,20 @@ func TestAclArrayDecoding(t *testing.T) { defer closeConn(t, conn) sql := "select $1::aclitem[]" + var scan []pgx.AclItem + tests := []struct { query []pgx.AclItem - scan []pgx.AclItem }{ { []pgx.AclItem{"=r/postgres"}, - []pgx.AclItem{}, }, { []pgx.AclItem{"=r/postgres", "postgres=arwdDxt/postgres"}, - []pgx.AclItem{}, }, } for i, tt := range tests { - err := conn.QueryRow(sql, tt.query).Scan(&tt.scan) + err := conn.QueryRow(sql, tt.query).Scan(&scan) if err != nil { t.Errorf(`%d. error reading array: %v`, i, err) if pgerr, ok := err.(pgx.PgError); ok { @@ -679,7 +678,7 @@ func TestAclArrayDecoding(t *testing.T) { } continue } - assertAclItemSlicesEqual(t, tt.query, tt.scan) + assertAclItemSlicesEqual(t, tt.query, scan) ensureConnValid(t, conn) } } From 9b8e3043baf38a70b943e89e22b7b61a77a0c7f7 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Sat, 12 Nov 2016 12:46:31 -0500 Subject: [PATCH 09/21] Handles empty aclitems --- values.go | 6 ++++++ values_test.go | 3 +++ 2 files changed, 9 insertions(+) diff --git a/values.go b/values.go index 395b59ce..ff2dfbfb 100644 --- a/values.go +++ b/values.go @@ -3022,6 +3022,12 @@ func decodeAclItemArray(vr *ValueReader) []AclItem { } str := vr.ReadString(vr.Len()) + + // short-circuit empty array + if str == "{}" { + return []AclItem{} + } + // remove the '{' at the front and the '}' at the end str = str[1 : len(str)-1] strs := strings.Split(str, ",") diff --git a/values_test.go b/values_test.go index 17a98197..4cbe40ea 100644 --- a/values_test.go +++ b/values_test.go @@ -662,6 +662,9 @@ func TestAclArrayDecoding(t *testing.T) { tests := []struct { query []pgx.AclItem }{ + { + []pgx.AclItem{}, + }, { []pgx.AclItem{"=r/postgres"}, }, From 4ba4d0097a929af934958aedc052e8302b3a484f Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Sun, 13 Nov 2016 18:08:36 -0500 Subject: [PATCH 10/21] Gets formatting correct for tricky ingoing string ...but broken for outgoing string; must fix next --- values_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/values_test.go b/values_test.go index 4cbe40ea..01e5114b 100644 --- a/values_test.go +++ b/values_test.go @@ -645,7 +645,7 @@ func TestNullX(t *testing.T) { func assertAclItemSlicesEqual(t *testing.T, query, scan []pgx.AclItem) { if !reflect.DeepEqual(query, scan) { - t.Errorf("failed to encode aclitem[]\n EXPECTED: %v\n ACTUAL: %v", query, scan) + t.Errorf("failed to encode aclitem[]\n EXPECTED: %d %v\n ACTUAL: %d %v", len(query), query, len(scan), scan) } } @@ -671,6 +671,9 @@ func TestAclArrayDecoding(t *testing.T) { { []pgx.AclItem{"=r/postgres", "postgres=arwdDxt/postgres"}, }, + { + []pgx.AclItem{"=r/postgres", "postgres=arwdDxt/postgres", `postgres=arwdDxt/\" tricky\, ' \} \"\" \\ test user \"`}, + }, } for i, tt := range tests { err := conn.QueryRow(sql, tt.query).Scan(&scan) From 5712d02e1b58313997f84705de92966505584210 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Tue, 15 Nov 2016 21:53:22 -0500 Subject: [PATCH 11/21] Gets tricky acl parsing working --- values.go | 152 ++++++++++++++++++++++++++++++++++++++++++++++++- values_test.go | 5 +- 2 files changed, 153 insertions(+), 4 deletions(-) diff --git a/values.go b/values.go index ff2dfbfb..fe8f82fa 100644 --- a/values.go +++ b/values.go @@ -5,6 +5,7 @@ import ( "database/sql/driver" "encoding/json" "fmt" + "io" "math" "net" "reflect" @@ -2999,12 +3000,42 @@ func decodeTextArray(vr *ValueReader) []string { return a } +func EscapeAclItem(acl string) (string, error) { + var buf bytes.Buffer + r := strings.NewReader(acl) + for { + rn, _, err := r.ReadRune() + if err != nil { + if err == io.EOF { + // This error was expected and is OK + return buf.String(), nil + } + // This error was not expected + return "", err + } + if NeedsEscape(rn) { + buf.WriteRune('\\') + } + buf.WriteRune(rn) + } +} + +func NeedsEscape(rn rune) bool { + return rn == '\\' || rn == ',' || rn == '"' || rn == '}' +} + // XXX: encodeAclItemSlice; using text encoding, not binary func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { // cast aclitems into strings so we can use strings.Join strs := make([]string, len(aclitems)) + var escaped string + var err error for i := range strs { - strs[i] = string(aclitems[i]) + escaped, err = EscapeAclItem(string(aclitems[i])) + if err != nil { + return err + } + strs[i] = string(escaped) } str := strings.Join(strs, ",") @@ -3014,6 +3045,121 @@ func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { return nil } +func ParseAclItemArray(arr string) ([]string, error) { + r := strings.NewReader(arr) + // Difficult to guess a performant initial capacity for a slice of + // values, but let's go with 5. + vals := make([]string, 0, 5) + // A single value + vlu := "" + for { + // Grab the first/next/last rune to see if we are dealing with a + // quoted value, an unquoted value, or the end of the string. + rn, _, err := r.ReadRune() + if err != nil { + if err == io.EOF { + // This error was expected and is OK + return vals, nil + } + // This error was not expected + return nil, err + } + + if rn == '"' { + // Discard the opening quote of the quoted value. + vlu, err = ParseQuotedAclItem(r) + } else { + // We have just read the first rune of an unquoted (bare) value; + // put it back so that ParseBareValue can read it. + err := r.UnreadRune() + if err != nil { + // This error was not expected. + return nil, err + } + vlu, err = ParseBareAclItem(r) + } + + if err != nil { + if err == io.EOF { + // This error was expected and is OK. + vals = append(vals, vlu) + return vals, nil + } + // This error was not expected. + return nil, err + } + vals = append(vals, vlu) + } +} + +func ParseBareAclItem(r *strings.Reader) (string, error) { + var buf bytes.Buffer + for { + rn, _, err := r.ReadRune() + if err != nil { + // Return the read value in case the error is a harmless io.EOF. + // (io.EOF marks the end of a bare value at the end of a string) + return buf.String(), err + } + if rn == ',' { + // A comma marks the end of a bare value. + return buf.String(), nil + } else { + buf.WriteRune(rn) + } + } +} + +func ParseQuotedAclItem(r *strings.Reader) (string, error) { + var buf bytes.Buffer + for { + rn, escaped, err := ReadPossiblyEscapedRune(r) + if err != nil { + if err == io.EOF { + // Even when it is the last value, the final rune of + // a quoted value should be the final closing quote, not io.EOF. + return "", fmt.Errorf("unexpected end of quoted value") + } + // Return the read value in case the error is a harmless io.EOF. + return buf.String(), err + } + if !escaped && rn == '"' { + // An unescaped double quote marks the end of a quoted value. + // The next rune should either be a comma or the end of the string. + rn, _, err := r.ReadRune() + if err != nil { + // Return the read value in case the error is a harmless io.EOF. + return buf.String(), err + } + if rn != ',' { + return "", fmt.Errorf("unexpected rune after quoted value") + } + return buf.String(), nil + } + buf.WriteRune(rn) + } +} + +// Returns the next rune from r, unless it is a backslash; +// in that case, it returns the rune after the backslash. The second +// return value tells us whether or not the rune was +// preceeded by a backslash (escaped). +func ReadPossiblyEscapedRune(r *strings.Reader) (rune, bool, error) { + rn, _, err := r.ReadRune() + if err != nil { + return 0, false, err + } + if rn == '\\' { + // Discard the backslash and read the next rune. + rn, _, err = r.ReadRune() + if err != nil { + return 0, false, err + } + return rn, true, nil + } + return rn, false, nil +} + // XXX: decodeAclItemArray; using text encoding, not binary func decodeAclItemArray(vr *ValueReader) []AclItem { if vr.Len() == -1 { @@ -3030,7 +3176,9 @@ func decodeAclItemArray(vr *ValueReader) []AclItem { // remove the '{' at the front and the '}' at the end str = str[1 : len(str)-1] - strs := strings.Split(str, ",") + strs, _ := ParseAclItemArray(str) + // XXX: what do I do with the error here? + // XXX strs := strings.Split(str, ",") // cast strings into AclItems before returning aclitems := make([]AclItem, len(strs)) diff --git a/values_test.go b/values_test.go index 01e5114b..8c5d1032 100644 --- a/values_test.go +++ b/values_test.go @@ -672,13 +672,14 @@ func TestAclArrayDecoding(t *testing.T) { []pgx.AclItem{"=r/postgres", "postgres=arwdDxt/postgres"}, }, { - []pgx.AclItem{"=r/postgres", "postgres=arwdDxt/postgres", `postgres=arwdDxt/\" tricky\, ' \} \"\" \\ test user \"`}, + []pgx.AclItem{"=r/postgres", "postgres=arwdDxt/postgres", `postgres=arwdDxt/" tricky, ' } "" \ test user "`}, }, } for i, tt := range tests { err := conn.QueryRow(sql, tt.query).Scan(&scan) if err != nil { - t.Errorf(`%d. error reading array: %v`, i, err) + // t.Errorf(`%d. error reading array: %v`, i, err) + t.Errorf(`%d. error reading array: %v query: %s`, i, err, tt.query) if pgerr, ok := err.(pgx.PgError); ok { t.Errorf(`%d. error reading array (detail): %s`, i, pgerr.Detail) } From 6ec7e84dbfe77cf613aab7077cc532a0daeda80b Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Tue, 15 Nov 2016 22:05:52 -0500 Subject: [PATCH 12/21] Handles parse error for aclitem[] --- values.go | 8 +++++--- values_test.go | 1 - 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/values.go b/values.go index fe8f82fa..cf3a0266 100644 --- a/values.go +++ b/values.go @@ -3176,9 +3176,11 @@ func decodeAclItemArray(vr *ValueReader) []AclItem { // remove the '{' at the front and the '}' at the end str = str[1 : len(str)-1] - strs, _ := ParseAclItemArray(str) - // XXX: what do I do with the error here? - // XXX strs := strings.Split(str, ",") + strs, err := ParseAclItemArray(str) + if err != nil { + vr.Fatal(ProtocolError(err.Error())) + return nil + } // cast strings into AclItems before returning aclitems := make([]AclItem, len(strs)) diff --git a/values_test.go b/values_test.go index 8c5d1032..bbb22f24 100644 --- a/values_test.go +++ b/values_test.go @@ -649,7 +649,6 @@ func assertAclItemSlicesEqual(t *testing.T, query, scan []pgx.AclItem) { } } -// XXX func TestAclArrayDecoding(t *testing.T) { t.Parallel() From 1ebcbab8a30b72bb9568839794116ccac0121bf1 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Tue, 15 Nov 2016 22:09:55 -0500 Subject: [PATCH 13/21] Removes unneeded XXXs --- values.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/values.go b/values.go index cf3a0266..61aaeb81 100644 --- a/values.go +++ b/values.go @@ -3024,7 +3024,6 @@ func NeedsEscape(rn rune) bool { return rn == '\\' || rn == ',' || rn == '"' || rn == '}' } -// XXX: encodeAclItemSlice; using text encoding, not binary func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { // cast aclitems into strings so we can use strings.Join strs := make([]string, len(aclitems)) @@ -3160,7 +3159,6 @@ func ReadPossiblyEscapedRune(r *strings.Reader) (rune, bool, error) { return rn, false, nil } -// XXX: decodeAclItemArray; using text encoding, not binary func decodeAclItemArray(vr *ValueReader) []AclItem { if vr.Len() == -1 { vr.Fatal(ProtocolError("Cannot decode null into []AclItem")) From 7b3488b088492d493be71487c28209cb7b005feb Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Tue, 15 Nov 2016 22:14:08 -0500 Subject: [PATCH 14/21] Makes parseAclItemArray helpers private --- values.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/values.go b/values.go index 61aaeb81..d8ad8fb3 100644 --- a/values.go +++ b/values.go @@ -3044,7 +3044,7 @@ func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { return nil } -func ParseAclItemArray(arr string) ([]string, error) { +func parseAclItemArray(arr string) ([]string, error) { r := strings.NewReader(arr) // Difficult to guess a performant initial capacity for a slice of // values, but let's go with 5. @@ -3066,7 +3066,7 @@ func ParseAclItemArray(arr string) ([]string, error) { if rn == '"' { // Discard the opening quote of the quoted value. - vlu, err = ParseQuotedAclItem(r) + vlu, err = parseQuotedAclItem(r) } else { // We have just read the first rune of an unquoted (bare) value; // put it back so that ParseBareValue can read it. @@ -3075,7 +3075,7 @@ func ParseAclItemArray(arr string) ([]string, error) { // This error was not expected. return nil, err } - vlu, err = ParseBareAclItem(r) + vlu, err = parseBareAclItem(r) } if err != nil { @@ -3091,7 +3091,7 @@ func ParseAclItemArray(arr string) ([]string, error) { } } -func ParseBareAclItem(r *strings.Reader) (string, error) { +func parseBareAclItem(r *strings.Reader) (string, error) { var buf bytes.Buffer for { rn, _, err := r.ReadRune() @@ -3109,10 +3109,10 @@ func ParseBareAclItem(r *strings.Reader) (string, error) { } } -func ParseQuotedAclItem(r *strings.Reader) (string, error) { +func parseQuotedAclItem(r *strings.Reader) (string, error) { var buf bytes.Buffer for { - rn, escaped, err := ReadPossiblyEscapedRune(r) + rn, escaped, err := readPossiblyEscapedRune(r) if err != nil { if err == io.EOF { // Even when it is the last value, the final rune of @@ -3143,7 +3143,7 @@ func ParseQuotedAclItem(r *strings.Reader) (string, error) { // in that case, it returns the rune after the backslash. The second // return value tells us whether or not the rune was // preceeded by a backslash (escaped). -func ReadPossiblyEscapedRune(r *strings.Reader) (rune, bool, error) { +func readPossiblyEscapedRune(r *strings.Reader) (rune, bool, error) { rn, _, err := r.ReadRune() if err != nil { return 0, false, err @@ -3174,7 +3174,7 @@ func decodeAclItemArray(vr *ValueReader) []AclItem { // remove the '{' at the front and the '}' at the end str = str[1 : len(str)-1] - strs, err := ParseAclItemArray(str) + strs, err := parseAclItemArray(str) if err != nil { vr.Fatal(ProtocolError(err.Error())) return nil From 323e2b3f7817715c392cc848417c485bf5564bfd Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Tue, 15 Nov 2016 22:22:57 -0500 Subject: [PATCH 15/21] Adds aclitem helper func tests --- aclitem_parse_test.go | 126 ++++++++++++++++++++++++++++++++++++++++++ values.go | 4 +- 2 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 aclitem_parse_test.go diff --git a/aclitem_parse_test.go b/aclitem_parse_test.go new file mode 100644 index 00000000..a0e5e858 --- /dev/null +++ b/aclitem_parse_test.go @@ -0,0 +1,126 @@ +package pgx + +import ( + "reflect" + "testing" +) + +func TestEscapeAclItem(t *testing.T) { + tests := []struct { + input string + expected string + }{ + { + "foo", + "foo", + }, + { + `foo, "\}`, + `foo\, \"\\\}`, + }, + } + + for i, tt := range tests { + actual, err := escapeAclItem(tt.input) + + if err != nil { + t.Errorf("%d. Unexpected error %v", i, err) + } + + if actual != tt.expected { + t.Errorf("%d.\nexpected: %s,\nactual: %s", i, tt.expected, actual) + } + } +} + +func TestParseAclItemArray(t *testing.T) { + tests := []struct { + input string + expected []string + errMsg string + }{ + { + "", + []string{}, + "", + }, + { + "one", + []string{"one"}, + "", + }, + { + `"one"`, + []string{"one"}, + "", + }, + { + "one,two,three", + []string{"one", "two", "three"}, + "", + }, + { + `"one","two","three"`, + []string{"one", "two", "three"}, + "", + }, + { + `"one",two,"three"`, + []string{"one", "two", "three"}, + "", + }, + { + `one,two,"three"`, + []string{"one", "two", "three"}, + "", + }, + { + `"one","two",three`, + []string{"one", "two", "three"}, + "", + }, + { + `"one","t w o",three`, + []string{"one", "t w o", "three"}, + "", + }, + { + `"one","t, w o\"\}\\",three`, + []string{"one", `t, w o"}\`, "three"}, + "", + }, + { + `"one","two",three"`, + []string{"one", "two", `three"`}, + "", + }, + { + `"one","two,"three"`, + nil, + "unexpected rune after quoted value", + }, + { + `"one","two","three`, + nil, + "unexpected end of quoted value", + }, + } + + for i, tt := range tests { + actual, err := parseAclItemArray(tt.input) + + if err != nil { + if tt.errMsg == "" { + t.Errorf("%d. Unexpected error %v", i, err) + } else if err.Error() != tt.errMsg { + t.Errorf("%d. Expected error %v did not match actual error %v", i, tt.errMsg, err.Error()) + } + } else if tt.errMsg != "" { + t.Errorf("%d. Expected error not returned: \"%v\"", i, tt.errMsg) + } + + if !reflect.DeepEqual(actual, tt.expected) { + t.Errorf("%d. Expected %v did not match actual %v", i, tt.expected, actual) + } + } +} diff --git a/values.go b/values.go index d8ad8fb3..a209ec80 100644 --- a/values.go +++ b/values.go @@ -3000,7 +3000,7 @@ func decodeTextArray(vr *ValueReader) []string { return a } -func EscapeAclItem(acl string) (string, error) { +func escapeAclItem(acl string) (string, error) { var buf bytes.Buffer r := strings.NewReader(acl) for { @@ -3030,7 +3030,7 @@ func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { var escaped string var err error for i := range strs { - escaped, err = EscapeAclItem(string(aclitems[i])) + escaped, err = escapeAclItem(string(aclitems[i])) if err != nil { return err } From 4b430a254ec0bfc251270e639b1099000711ac54 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Thu, 17 Nov 2016 21:38:00 -0500 Subject: [PATCH 16/21] Improves docs around aclitem[] --- values.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/values.go b/values.go index a209ec80..4312a61f 100644 --- a/values.go +++ b/values.go @@ -3000,6 +3000,12 @@ func decodeTextArray(vr *ValueReader) []string { return a } +// escapeAclItem escapes an AclItem before it is added to +// its aclitem[] string representation. The PostgreSQL aclitem +// datatype itself can need escapes because it follows the +// formatting rules of SQL identifiers. Think of this function +// as escaping the escapes, so that PostgreSQL's array parser +// will do the right thing. func escapeAclItem(acl string) (string, error) { var buf bytes.Buffer r := strings.NewReader(acl) @@ -3013,17 +3019,22 @@ func escapeAclItem(acl string) (string, error) { // This error was not expected return "", err } - if NeedsEscape(rn) { + if needsEscape(rn) { buf.WriteRune('\\') } buf.WriteRune(rn) } } -func NeedsEscape(rn rune) bool { +// needsEscape determines whether or not a rune needs escaping +// before being placed in the textual representation of an +// aclitem[] array. +func needsEscape(rn rune) bool { return rn == '\\' || rn == ',' || rn == '"' || rn == '}' } +// encodeAclItemSlice encodes a slice of AclItems in +// their textual represention for PostgreSQL. func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { // cast aclitems into strings so we can use strings.Join strs := make([]string, len(aclitems)) @@ -3044,10 +3055,12 @@ func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { return nil } +// parseAclItemArray parses the textual representation +// of the aclitem[] type. func parseAclItemArray(arr string) ([]string, error) { r := strings.NewReader(arr) // Difficult to guess a performant initial capacity for a slice of - // values, but let's go with 5. + // aclitems, but let's go with 5. vals := make([]string, 0, 5) // A single value vlu := "" From 3906f7c0d084c178ee8155abd9b8e570d5d43f25 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Thu, 17 Nov 2016 21:45:46 -0500 Subject: [PATCH 17/21] Casts aclitem earl to avoid O(2n) --- aclitem_parse_test.go | 24 ++++++++++++------------ values.go | 16 +++++----------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/aclitem_parse_test.go b/aclitem_parse_test.go index a0e5e858..5c7c748f 100644 --- a/aclitem_parse_test.go +++ b/aclitem_parse_test.go @@ -36,62 +36,62 @@ func TestEscapeAclItem(t *testing.T) { func TestParseAclItemArray(t *testing.T) { tests := []struct { input string - expected []string + expected []AclItem errMsg string }{ { "", - []string{}, + []AclItem{}, "", }, { "one", - []string{"one"}, + []AclItem{"one"}, "", }, { `"one"`, - []string{"one"}, + []AclItem{"one"}, "", }, { "one,two,three", - []string{"one", "two", "three"}, + []AclItem{"one", "two", "three"}, "", }, { `"one","two","three"`, - []string{"one", "two", "three"}, + []AclItem{"one", "two", "three"}, "", }, { `"one",two,"three"`, - []string{"one", "two", "three"}, + []AclItem{"one", "two", "three"}, "", }, { `one,two,"three"`, - []string{"one", "two", "three"}, + []AclItem{"one", "two", "three"}, "", }, { `"one","two",three`, - []string{"one", "two", "three"}, + []AclItem{"one", "two", "three"}, "", }, { `"one","t w o",three`, - []string{"one", "t w o", "three"}, + []AclItem{"one", "t w o", "three"}, "", }, { `"one","t, w o\"\}\\",three`, - []string{"one", `t, w o"}\`, "three"}, + []AclItem{"one", `t, w o"}\`, "three"}, "", }, { `"one","two",three"`, - []string{"one", "two", `three"`}, + []AclItem{"one", "two", `three"`}, "", }, { diff --git a/values.go b/values.go index 4312a61f..490d2f38 100644 --- a/values.go +++ b/values.go @@ -3057,11 +3057,11 @@ func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { // parseAclItemArray parses the textual representation // of the aclitem[] type. -func parseAclItemArray(arr string) ([]string, error) { +func parseAclItemArray(arr string) ([]AclItem, error) { r := strings.NewReader(arr) // Difficult to guess a performant initial capacity for a slice of // aclitems, but let's go with 5. - vals := make([]string, 0, 5) + vals := make([]AclItem, 0, 5) // A single value vlu := "" for { @@ -3094,13 +3094,13 @@ func parseAclItemArray(arr string) ([]string, error) { if err != nil { if err == io.EOF { // This error was expected and is OK. - vals = append(vals, vlu) + vals = append(vals, AclItem(vlu)) return vals, nil } // This error was not expected. return nil, err } - vals = append(vals, vlu) + vals = append(vals, AclItem(vlu)) } } @@ -3192,13 +3192,7 @@ func decodeAclItemArray(vr *ValueReader) []AclItem { vr.Fatal(ProtocolError(err.Error())) return nil } - - // cast strings into AclItems before returning - aclitems := make([]AclItem, len(strs)) - for i := range aclitems { - aclitems[i] = AclItem(strs[i]) - } - return aclitems + return strs } func encodeStringSlice(w *WriteBuf, oid Oid, slice []string) error { From bce83fd4ba2f29c5528fc688f49c9df952205c1f Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Thu, 17 Nov 2016 21:59:05 -0500 Subject: [PATCH 18/21] Better names and efficiency --- values.go | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/values.go b/values.go index 490d2f38..c10ef782 100644 --- a/values.go +++ b/values.go @@ -3007,22 +3007,22 @@ func decodeTextArray(vr *ValueReader) []string { // as escaping the escapes, so that PostgreSQL's array parser // will do the right thing. func escapeAclItem(acl string) (string, error) { - var buf bytes.Buffer - r := strings.NewReader(acl) + var escapedAclItem bytes.Buffer + reader := strings.NewReader(acl) for { - rn, _, err := r.ReadRune() + rn, _, err := reader.ReadRune() if err != nil { if err == io.EOF { - // This error was expected and is OK - return buf.String(), nil + // Here, EOF is an expected end state, not an error. + return escapedAclItem.String(), nil } // This error was not expected return "", err } if needsEscape(rn) { - buf.WriteRune('\\') + escapedAclItem.WriteRune('\\') } - buf.WriteRune(rn) + escapedAclItem.WriteRune(rn) } } @@ -3038,18 +3038,21 @@ func needsEscape(rn rune) bool { func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { // cast aclitems into strings so we can use strings.Join strs := make([]string, len(aclitems)) - var escaped string + var escapedAclItem string var err error for i := range strs { - escaped, err = escapeAclItem(string(aclitems[i])) + escapedAclItem, err = escapeAclItem(string(aclitems[i])) if err != nil { return err } - strs[i] = string(escaped) + strs[i] = string(escapedAclItem) } - str := strings.Join(strs, ",") - str = "{" + str + "}" + var buf bytes.Buffer + buf.WriteRune('{') + buf.WriteString(strings.Join(strs, ",")) + buf.WriteRune('}') + str := buf.String() w.WriteInt32(int32(len(str))) w.WriteBytes([]byte(str)) return nil @@ -3070,7 +3073,7 @@ func parseAclItemArray(arr string) ([]AclItem, error) { rn, _, err := r.ReadRune() if err != nil { if err == io.EOF { - // This error was expected and is OK + // Here, EOF is an expected end state, not an error. return vals, nil } // This error was not expected @@ -3093,7 +3096,7 @@ func parseAclItemArray(arr string) ([]AclItem, error) { if err != nil { if err == io.EOF { - // This error was expected and is OK. + // Here, EOF is an expected end state, not an error.. vals = append(vals, AclItem(vlu)) return vals, nil } From 09ee8a9b703ca995aacfd4540915e37b62edb005 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Thu, 17 Nov 2016 22:08:56 -0500 Subject: [PATCH 19/21] Returns AclItem earlier --- values.go | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/values.go b/values.go index c10ef782..58ac118f 100644 --- a/values.go +++ b/values.go @@ -3036,7 +3036,6 @@ func needsEscape(rn rune) bool { // encodeAclItemSlice encodes a slice of AclItems in // their textual represention for PostgreSQL. func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { - // cast aclitems into strings so we can use strings.Join strs := make([]string, len(aclitems)) var escapedAclItem string var err error @@ -3061,20 +3060,20 @@ func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { // parseAclItemArray parses the textual representation // of the aclitem[] type. func parseAclItemArray(arr string) ([]AclItem, error) { - r := strings.NewReader(arr) + reader := strings.NewReader(arr) // Difficult to guess a performant initial capacity for a slice of // aclitems, but let's go with 5. - vals := make([]AclItem, 0, 5) + aclItems := make([]AclItem, 0, 5) // A single value - vlu := "" + aclItem := AclItem("") for { // Grab the first/next/last rune to see if we are dealing with a // quoted value, an unquoted value, or the end of the string. - rn, _, err := r.ReadRune() + rn, _, err := reader.ReadRune() if err != nil { if err == io.EOF { // Here, EOF is an expected end state, not an error. - return vals, nil + return aclItems, nil } // This error was not expected return nil, err @@ -3082,50 +3081,49 @@ func parseAclItemArray(arr string) ([]AclItem, error) { if rn == '"' { // Discard the opening quote of the quoted value. - vlu, err = parseQuotedAclItem(r) + aclItem, err = parseQuotedAclItem(reader) } else { // We have just read the first rune of an unquoted (bare) value; // put it back so that ParseBareValue can read it. - err := r.UnreadRune() + err := reader.UnreadRune() if err != nil { - // This error was not expected. return nil, err } - vlu, err = parseBareAclItem(r) + aclItem, err = parseBareAclItem(reader) } if err != nil { if err == io.EOF { // Here, EOF is an expected end state, not an error.. - vals = append(vals, AclItem(vlu)) - return vals, nil + aclItems = append(aclItems, aclItem) + return aclItems, nil } // This error was not expected. return nil, err } - vals = append(vals, AclItem(vlu)) + aclItems = append(aclItems, aclItem) } } -func parseBareAclItem(r *strings.Reader) (string, error) { +func parseBareAclItem(r *strings.Reader) (AclItem, error) { var buf bytes.Buffer for { rn, _, err := r.ReadRune() if err != nil { // Return the read value in case the error is a harmless io.EOF. // (io.EOF marks the end of a bare value at the end of a string) - return buf.String(), err + return AclItem(buf.String()), err } if rn == ',' { // A comma marks the end of a bare value. - return buf.String(), nil + return AclItem(buf.String()), nil } else { buf.WriteRune(rn) } } } -func parseQuotedAclItem(r *strings.Reader) (string, error) { +func parseQuotedAclItem(r *strings.Reader) (AclItem, error) { var buf bytes.Buffer for { rn, escaped, err := readPossiblyEscapedRune(r) @@ -3133,10 +3131,10 @@ func parseQuotedAclItem(r *strings.Reader) (string, error) { if err == io.EOF { // Even when it is the last value, the final rune of // a quoted value should be the final closing quote, not io.EOF. - return "", fmt.Errorf("unexpected end of quoted value") + return AclItem(""), fmt.Errorf("unexpected end of quoted value") } // Return the read value in case the error is a harmless io.EOF. - return buf.String(), err + return AclItem(buf.String()), err } if !escaped && rn == '"' { // An unescaped double quote marks the end of a quoted value. @@ -3144,12 +3142,12 @@ func parseQuotedAclItem(r *strings.Reader) (string, error) { rn, _, err := r.ReadRune() if err != nil { // Return the read value in case the error is a harmless io.EOF. - return buf.String(), err + return AclItem(buf.String()), err } if rn != ',' { - return "", fmt.Errorf("unexpected rune after quoted value") + return AclItem(""), fmt.Errorf("unexpected rune after quoted value") } - return buf.String(), nil + return AclItem(buf.String()), nil } buf.WriteRune(rn) } From 7bd2e85f31660a6fc200b064c8c8d99b0ba3974f Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Thu, 17 Nov 2016 22:18:09 -0500 Subject: [PATCH 20/21] Improves names and comments --- values.go | 57 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/values.go b/values.go index 58ac118f..77ae0bca 100644 --- a/values.go +++ b/values.go @@ -3105,51 +3105,55 @@ func parseAclItemArray(arr string) ([]AclItem, error) { } } -func parseBareAclItem(r *strings.Reader) (AclItem, error) { - var buf bytes.Buffer +// parseBareAclItem parses a bare (unquoted) aclitem from reader +func parseBareAclItem(reader *strings.Reader) (AclItem, error) { + var aclItem bytes.Buffer for { - rn, _, err := r.ReadRune() + rn, _, err := reader.ReadRune() if err != nil { // Return the read value in case the error is a harmless io.EOF. - // (io.EOF marks the end of a bare value at the end of a string) - return AclItem(buf.String()), err + // (io.EOF marks the end of a bare aclitem at the end of a string) + return AclItem(aclItem.String()), err } if rn == ',' { - // A comma marks the end of a bare value. - return AclItem(buf.String()), nil + // A comma marks the end of a bare aclitem. + return AclItem(aclItem.String()), nil } else { - buf.WriteRune(rn) + aclItem.WriteRune(rn) } } } -func parseQuotedAclItem(r *strings.Reader) (AclItem, error) { - var buf bytes.Buffer +// parseQuotedAclItem parses an aclitem which is in double quotes from reader +func parseQuotedAclItem(reader *strings.Reader) (AclItem, error) { + var aclItem bytes.Buffer for { - rn, escaped, err := readPossiblyEscapedRune(r) + rn, escaped, err := readPossiblyEscapedRune(reader) if err != nil { if err == io.EOF { // Even when it is the last value, the final rune of - // a quoted value should be the final closing quote, not io.EOF. + // a quoted aclitem should be the final closing quote, not io.EOF. return AclItem(""), fmt.Errorf("unexpected end of quoted value") } - // Return the read value in case the error is a harmless io.EOF. - return AclItem(buf.String()), err + // Return the read aclitem in case the error is a harmless io.EOF, + // which will be determined by the caller. + return AclItem(aclItem.String()), err } if !escaped && rn == '"' { // An unescaped double quote marks the end of a quoted value. // The next rune should either be a comma or the end of the string. - rn, _, err := r.ReadRune() + rn, _, err := reader.ReadRune() if err != nil { - // Return the read value in case the error is a harmless io.EOF. - return AclItem(buf.String()), err + // Return the read value in case the error is a harmless io.EOF, + // which will be determined by the caller. + return AclItem(aclItem.String()), err } if rn != ',' { return AclItem(""), fmt.Errorf("unexpected rune after quoted value") } - return AclItem(buf.String()), nil + return AclItem(aclItem.String()), nil } - buf.WriteRune(rn) + aclItem.WriteRune(rn) } } @@ -3157,14 +3161,14 @@ func parseQuotedAclItem(r *strings.Reader) (AclItem, error) { // in that case, it returns the rune after the backslash. The second // return value tells us whether or not the rune was // preceeded by a backslash (escaped). -func readPossiblyEscapedRune(r *strings.Reader) (rune, bool, error) { - rn, _, err := r.ReadRune() +func readPossiblyEscapedRune(reader *strings.Reader) (rune, bool, error) { + rn, _, err := reader.ReadRune() if err != nil { return 0, false, err } if rn == '\\' { // Discard the backslash and read the next rune. - rn, _, err = r.ReadRune() + rn, _, err = reader.ReadRune() if err != nil { return 0, false, err } @@ -3181,19 +3185,20 @@ func decodeAclItemArray(vr *ValueReader) []AclItem { str := vr.ReadString(vr.Len()) - // short-circuit empty array + // Short-circuit empty array. if str == "{}" { return []AclItem{} } - // remove the '{' at the front and the '}' at the end + // Remove the '{' at the front and the '}' at the end, + // so that parseAclItemArray doesn't have to deal with them. str = str[1 : len(str)-1] - strs, err := parseAclItemArray(str) + aclItems, err := parseAclItemArray(str) if err != nil { vr.Fatal(ProtocolError(err.Error())) return nil } - return strs + return aclItems } func encodeStringSlice(w *WriteBuf, oid Oid, slice []string) error { From 3beac831cf40e5361fa4edda0b2d25af91e14ff3 Mon Sep 17 00:00:00 2001 From: Manni Wood Date: Thu, 17 Nov 2016 22:25:00 -0500 Subject: [PATCH 21/21] Adds formatting notes --- values.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/values.go b/values.go index 77ae0bca..8a7a49cb 100644 --- a/values.go +++ b/values.go @@ -3058,7 +3058,10 @@ func encodeAclItemSlice(w *WriteBuf, oid Oid, aclitems []AclItem) error { } // parseAclItemArray parses the textual representation -// of the aclitem[] type. +// of the aclitem[] type. The textual representation is chosen because +// Pg's src/backend/utils/adt/acl.c has only in/out (text) not send/recv (bin). +// See https://www.postgresql.org/docs/current/static/arrays.html#ARRAYS-IO +// for formatting notes. func parseAclItemArray(arr string) ([]AclItem, error) { reader := strings.NewReader(arr) // Difficult to guess a performant initial capacity for a slice of