fix(datastore): remove namespace from Key.String() (#10684) (#10823)

* fix(datastore): remove namespace from Key.String() (#10684)

Datastore namespaces may be sensitive, and it's best not to emit
them.

Provide a `datastore.Key.StringWithNamespace()` as an alternative,
and use this version internally, which preserves the fix from
#7829.

* amend! fix(datastore): remove namespace from Key.String() (#10684)

fix(datastore): remove namespace from Key.String()

Fixes #10684.

Datastore namespaces may be sensitive, and it's best not to emit them.

Restores the behavior of `Key.String` prior to #8363, but maintains the
fix for #7829 by providing an internal implementation that does provide
the namespace.

Author: sgp

datastore/datastore.go

@@ -540,7 +540,7 @@ func (c *Client) get(ctx context.Context, keys []*Key, dst interface{}, opts *pb
 			multiErr[i] = fmt.Errorf("datastore: can't get the incomplete key: %v", k)
 			any = true
 		} else {
-			ks := k.String()
+			ks := k.stringInternal()
 			if _, ok := keyMap[ks]; !ok {
 				pbKeys = append(pbKeys, keyToProto(k))
 			}
@@ -584,8 +584,8 @@ func (c *Client) get(ctx context.Context, keys []*Key, dst interface{}, opts *pb
 		if err != nil {
 			return txnID, errors.New("datastore: internal error: server returned an invalid key")
 		}
-		filled += len(keyMap[k.String()])
-		for _, index := range keyMap[k.String()] {
+		filled += len(keyMap[k.stringInternal()])
+		for _, index := range keyMap[k.stringInternal()] {
 			elem := v.Index(index)
 			if multiArgType == multiArgTypePropertyLoadSaver || multiArgType == multiArgTypeStruct {
 				elem = elem.Addr()
@@ -604,8 +604,8 @@ func (c *Client) get(ctx context.Context, keys []*Key, dst interface{}, opts *pb
 		if err != nil {
 			return txnID, errors.New("datastore: internal error: server returned an invalid key")
 		}
-		filled += len(keyMap[k.String()])
-		for _, index := range keyMap[k.String()] {
+		filled += len(keyMap[k.stringInternal()])
+		for _, index := range keyMap[k.stringInternal()] {
 			multiErr[index] = ErrNoSuchEntity
 		}
 		any = true
@@ -798,7 +798,7 @@ func deleteMutations(keys []*Key) ([]*pb.Mutation, error) {
 			multiErr[i] = fmt.Errorf("datastore: can't delete the incomplete key: %v", k)
 			hasErr = true
 		} else {
-			ks := k.String()
+			ks := k.stringInternal()
 			if !set[ks] {
 				mutations = append(mutations, &pb.Mutation{
 					Operation: &pb.Mutation_Delete{Delete: keyToProto(k)},

datastore/key.go

@@ -93,9 +93,10 @@ func (k *Key) Equal(o *Key) bool {
 }
 
 // marshal marshals the key's string representation to the buffer.
-func (k *Key) marshal(b *bytes.Buffer) {
+// If includeSensitive is true, it will include the namespace when creating the string.
+func (k *Key) marshal(b *bytes.Buffer, includeSensitive bool) {
 	if k.Parent != nil {
-		k.Parent.marshal(b)
+		k.Parent.marshal(b, includeSensitive)
 	}
 	b.WriteByte('/')
 	b.WriteString(k.Kind)
@@ -105,19 +106,29 @@ func (k *Key) marshal(b *bytes.Buffer) {
 	} else {
 		b.WriteString(strconv.FormatInt(k.ID, 10))
 	}
-	if k.Namespace != "" {
+	if k.Namespace != "" && includeSensitive {
 		b.WriteByte(',')
 		b.WriteString(k.Namespace)
 	}
 }
 
-// String returns a string representation of the key.
+// String returns a string representation of the key. It does not include
+// the namespace in case that the Namespace's name is sensitive information.
 func (k *Key) String() string {
+	return k.string(false)
+}
+
+// stringInternal is identical to String(), but appends the namespace.
+func (k *Key) stringInternal() string {
+	return k.string(true)
+}
+
+func (k *Key) string(includeSensitive bool) string {
 	if k == nil {
 		return ""
 	}
 	b := bytes.NewBuffer(make([]byte, 0, 512))
-	k.marshal(b)
+	k.marshal(b, includeSensitive)
 	return b.String()
 }
 

datastore/mutation.go

@@ -122,7 +122,7 @@ func mutationProtos(muts []*Mutation) ([]*pb.Mutation, error) {
 	seen := map[string]bool{}
 	for _, m := range muts {
 		if m.isDelete() {
-			ks := m.key.String()
+			ks := m.key.stringInternal()
 			if seen[ks] {
 				continue
 			}