GT-510 Fix using VST for database with non-ANSI characters (#563)

Author: jwierzbo

CHANGELOG.md

@@ -27,6 +27,7 @@
 - Move examples to separate package
 - Deprecate ClientConfig.SynchronizeEndpointsInterval due to bug in implementation
 - [V1] Add Rename function for collections (single server only).
+- [V1] Fix using VST for database with non-ANSI characters
 
 ## [1.6.0](https://github.com/arangodb/go-driver/tree/v1.6.0) (2023-05-30)
 - Add ErrArangoDatabaseNotFound and IsExternalStorageError helper to v2

test/collection_test.go

@@ -23,6 +23,7 @@ package test
 import (
 	"context"
 	"fmt"
+	"strconv"
 	"testing"
 	"time"
 
@@ -211,19 +212,10 @@ func TestCollection_ComputedValues(t *testing.T) {
 		createdAtValue, createdAtIsPresent := readDoc["createdAt"]
 		require.True(t, createdAtIsPresent)
 
-		// Verify that the computed value is a valid date
-		var createdAtValueInt64 int64
-
-		switch cav := createdAtValue.(type) {
-		case int64:
-			createdAtValueInt64 = cav
-		case float64:
-			createdAtValueInt64 = int64(cav)
-		case uint64:
-			createdAtValueInt64 = int64(cav)
-		default:
-			t.Fatalf("Unexpected type of createdAt value: %T", createdAtValue)
-		}
+		t.Logf("createdAtValue raw value: %v", createdAtValue)
+		createdAtValueInt64, err := parseInt64FromInterface(createdAtValue)
+		require.NoError(t, err)
+		t.Logf("createdAtValue parsed value: %v", createdAtValueInt64)
 
 		tm := time.Unix(createdAtValueInt64, 0)
 		require.True(t, tm.After(time.Now().Add(-time.Second)))
@@ -415,6 +407,23 @@ func TestCreateCollectionWithShardingStrategy(t *testing.T) {
 	}
 }
 
+func parseInt64FromInterface(value interface{}) (int64, error) {
+	switch v := value.(type) {
+	case int:
+		return int64(v), nil
+	case int8, int16, int32, int64:
+		return v.(int64), nil
+	case uint, uint8, uint16, uint32, uint64:
+		return int64(v.(uint64)), nil
+	case float32, float64:
+		return int64(v.(float64)), nil
+	case string:
+		return strconv.ParseInt(v, 10, 64)
+	default:
+		return 0, fmt.Errorf("value is of type %T, not convertible to int64", v)
+	}
+}
+
 // TestRemoveCollection creates a collection and then removes it.
 func TestRemoveCollection(t *testing.T) {
 	c := createClient(t, nil)

test/database_test.go

@@ -27,6 +27,7 @@ import (
 	"testing"
 
 	"github.com/dchest/uniuri"
+	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/text/unicode/norm"
 
@@ -167,7 +168,7 @@ func TestDatabaseNameUnicode(t *testing.T) {
 	c := createClient(t, nil)
 	databaseExtendedNamesRequired(t, c)
 
-	dbName := "\u006E\u0303\u00f1"
+	dbName := "\u006E\u0303\u00f1" + uuid.New().String()
 	normalized := norm.NFC.String(dbName)
 	ctx := context.Background()
 	_, err := c.CreateDatabase(ctx, dbName, nil)

v2/tests/database_collection_operations_test.go

@@ -512,7 +512,7 @@ func TestDatabaseNameUnicode(t *testing.T) {
 // with the option --database.extended-names-databases=true.
 func databaseExtendedNamesRequired(t *testing.T, c arangodb.Client, ctx context.Context) {
 	// If the database can be created with the below name then it means that it excepts unicode names.
-	dbName := "\u006E\u0303\u00f1"
+	dbName := "\u006E\u0303\u00f1" + uuid.New().String()
 	normalized := norm.NFC.String(dbName)
 	db, err := c.CreateDatabase(ctx, normalized, nil)
 	if err == nil {

vst/request.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2017 ArangoDB GmbH, Cologne, Germany
+// Copyright 2017-2023 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,8 +17,6 @@
 //
 // Copyright holder is ArangoDB GmbH, Cologne, Germany
 //
-// Author Ewout Prangsma
-//
 
 package vst
 
@@ -149,11 +147,17 @@ func (r *vstRequest) createMessageParts() ([][]byte, error) {
 	if strings.HasPrefix(path, "_db/") {
 		path = path[4:] // Remove '_db/'
 		parts := strings.SplitN(path, "/", 2)
+
+		// ensure database name is not URL-encoded
+		dbName, err := url.QueryUnescape(parts[0])
+		if err != nil {
+			return nil, driver.WithStack(err)
+		}
+		databaseValue = velocypack.NewStringValue(dbName)
+
 		if len(parts) == 1 {
-			databaseValue = velocypack.NewStringValue(parts[0])
 			path = ""
 		} else {
-			databaseValue = velocypack.NewStringValue(parts[0])
 			path = parts[1]
 		}
 	}
@@ -162,18 +166,34 @@ func (r *vstRequest) createMessageParts() ([][]byte, error) {
 	// Create header
 	var b velocypack.Builder
 	b.OpenArray()
-	b.AddValue(velocypack.NewIntValue(1))               // Version
-	b.AddValue(velocypack.NewIntValue(1))               // Type (1=Req)
-	b.AddValue(databaseValue)                           // Database name
-	b.AddValue(velocypack.NewIntValue(r.requestType())) // Request type
-	b.AddValue(velocypack.NewStringValue(path))         // Request
-	b.OpenObject()                                      // Parameters
+
+	// member 0: numeric version of the velocypack protocol. Must always be 1 at the moment.
+	b.AddValue(velocypack.NewIntValue(1))
+
+	// member 1: numeric representation of the VST request type. Must always be 1 at the moment.
+	b.AddValue(velocypack.NewIntValue(1))
+
+	// member 2: string with the database name - this must be the normalized database name, but not URL-encoded in any way!
+	b.AddValue(databaseValue) // Database name
+
+	// member 3: numeric representation of the request type (GET, POST, PUT etc.)
+	b.AddValue(velocypack.NewIntValue(r.requestType()))
+
+	// member 4: string with a relative request path, starting at /
+	// There is no need for this path to contain the database name, as the database name is already transferred in member 2.
+	// There is also no need for the path to contain request parameters (e.g. key=value), as they should be transferred in member 5.
+	b.AddValue(velocypack.NewStringValue(path))
+
+	// member 5:  object with request parameters (e.g. { "foo": "bar", "baz": "qux" }
+	b.OpenObject()
 	for k, v := range r.q {
 		if len(v) > 0 {
 			b.AddKeyValue(k, velocypack.NewStringValue(v[0]))
 		}
 	}
-	b.Close()      // Parameters
+	b.Close()
+
+	// member 6: object with “HTTP” headers (e.g. { "x-arango-async" : "store" }
 	b.OpenObject() // Meta
 	for k, v := range r.hdr {
 		b.AddKeyValue(k, velocypack.NewStringValue(v))