Improved NewClient endpoint handling

Made regular expression very strict. Gives false positives only for paths like
"/v20./".

Return approppriate errors for invalid version or if endpoint has a suffix.
Path must not continue after matched version.

Author: Markus Suonto

openstack/client.go

@@ -17,7 +17,7 @@ const (
 	v30 = "v3.0"
 )
 
-var VERSION_PATTERN = regexp.MustCompile("/v[1-9]{1}[0-9.]*")
+var VERSION_PATTERN = regexp.MustCompile("/v[1-9]{1}[0-9\\.]{0,2}/")
 
 // NewClient prepares an unauthenticated ProviderClient instance.
 // Most users will probably prefer using the AuthenticatedClient function instead.
@@ -34,16 +34,24 @@ func NewClient(endpoint string) (*gophercloud.ProviderClient, error) {
 	// Base is url with path
 	endpoint = gophercloud.NormalizeURL(endpoint)
 	base := gophercloud.NormalizeURL(u.String())
-
 	var location = VERSION_PATTERN.FindStringIndex(base)
 
-	// If version found remove it from base
 	if location != nil {
-		base = base[0:location[0]+1]
-		return &gophercloud.ProviderClient{
-			IdentityBase:     base,
-			IdentityEndpoint: endpoint,
-		}, nil
+		var version = base[location[0]+1:location[1]-1]
+		switch version {
+		case "v2.0", "v3":
+			// post version suffixes in path are not supported
+			if len(base) > location[1] {
+				return nil, fmt.Errorf("Path suffixes (after version) are not supported.")
+			}
+			// valid version found, strip from base
+			return &gophercloud.ProviderClient{
+				IdentityBase:     base[0:location[0]+1],
+				IdentityEndpoint: endpoint,
+			}, nil
+		default:
+			return nil, fmt.Errorf("Invalid identity endpoint version %v. Supported versions: v2.0, v3", version)
+		}
 	}
 
 	return &gophercloud.ProviderClient{

openstack/client_test.go

@@ -200,4 +200,54 @@ func TestNewClient(t *testing.T) {
 	th.AssertNoErr(t, err)
 	th.AssertEquals(t, "https://example.com/foo/bar/v2.0/", client.IdentityEndpoint)
 	th.AssertEquals(t, "https://example.com/foo/bar/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/foo/bar/v3/")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "https://example.com/foo/bar/v3/", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/foo/bar/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/v3")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "https://example.com/v3/", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/v3/")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "https://example.com/v3/", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/", client.IdentityBase)
+
+	_, err = NewClient("https://example.com/v2.3/")
+	th.AssertErr(t, err)
+	expected := fmt.Errorf("Invalid identity endpoint version %v. Supported versions: v2.0, v3", "v2.3")
+	th.AssertEquals(t, expected.Error(), err.Error())
+
+	_, err = NewClient("https://example.com/v2.0/foo")
+	th.AssertErr(t, err)
+	expected = fmt.Errorf("Path suffixes (after version) are not supported.")
+	th.AssertEquals(t, expected.Error(), err.Error())
+
+	// Does not match regexp, include to base
+	client, err = NewClient("https://example.com/v2a0/")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/v2a0/", client.IdentityBase)
+
+	// Does not match regexp, include to base
+	client, err = NewClient("https://example.com/v3api")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/v3api/", client.IdentityBase)
+
+	// Matches regexp, invalid version v20.
+	_, err = NewClient("https://example.com/v20./")
+	th.AssertErr(t, err)
+	expected = fmt.Errorf("Invalid identity endpoint version %v. Supported versions: v2.0, v3", "v20.")
+	th.AssertEquals(t, expected.Error(), err.Error())
+
+	// domain contains version
+	client, err = NewClient("https://v3.v2.0example.com/v3/")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "https://v3.v2.0example.com/v3/", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://v3.v2.0example.com/", client.IdentityBase)
+
 }

testhelper/convenience.go

@@ -224,6 +224,21 @@ func CheckEquals(t *testing.T, expected, actual interface{}) {
 	}
 }
 
+// AssertErr is a convenience function for checking that an error occurred
+func AssertErr(t *testing.T, e error) {
+	if e == nil {
+		logFatal(t, fmt.Sprintf("expected an error but none occurred"))
+	}
+}
+
+// CheckErr is a convenience function for checking that an error occurred,
+// except with a non-fatal error
+func CheckErr(t *testing.T, e error) {
+	if e == nil {
+		logError(t, fmt.Sprintf("expected an error but none occurred"))
+	}
+}
+
 // AssertDeepEquals - like Equals - performs a comparison - but on more complex
 // structures that requires deeper inspection
 func AssertDeepEquals(t *testing.T, expected, actual interface{}) {