feat: client side change id validation (#404)

Add client-side validation for change ID used in URLs.

Closes #318.

client/changes.go

@@ -19,6 +19,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/url"
+	"regexp"
 	"time"
 )
 
@@ -86,8 +87,18 @@ type changeAndData struct {
 	Data map[string]*json.RawMessage `json:"data"`
 }
 
+// This is used to ensure we send a well-formed change ID in the URL path.
+// It's a little more permissive than the currently-valid change IDs (which
+// are always integers), but it will allow older clients to talk to newer
+// servers which might start allowing letters too (for example).
+var changeIDRegexp = regexp.MustCompile(`^[a-z0-9]+$`)
+
 // Change fetches information about a Change given its ID.
 func (client *Client) Change(id string) (*Change, error) {
+	if !changeIDRegexp.MatchString(id) {
+		return nil, fmt.Errorf("invalid change ID %q", id)
+	}
+
 	var chgd changeAndData
 	_, err := client.doSync("GET", "/v1/changes/"+id, nil, nil, nil, &chgd)
 	if err != nil {
@@ -100,6 +111,10 @@ func (client *Client) Change(id string) (*Change, error) {
 
 // Abort attempts to abort a change that is not yet ready.
 func (client *Client) Abort(id string) (*Change, error) {
+	if !changeIDRegexp.MatchString(id) {
+		return nil, fmt.Errorf("invalid change ID %q", id)
+	}
+
 	var postData struct {
 		Action string `json:"action"`
 	}
@@ -183,6 +198,10 @@ type WaitChangeOptions struct {
 // succeeds, the returned Change.Err string will be non-empty if the change
 // itself had an error.
 func (client *Client) WaitChange(id string, opts *WaitChangeOptions) (*Change, error) {
+	if !changeIDRegexp.MatchString(id) {
+		return nil, fmt.Errorf("invalid change ID %q", id)
+	}
+
 	var chgd changeAndData
 
 	query := url.Values{}

client/changes_test.go

@@ -271,3 +271,18 @@ func (cs *clientSuite) TestClientAbort(c *check.C) {
 
 	c.Assert(string(body), check.Equals, "{\"action\":\"abort\"}\n")
 }
+
+func (cs *clientSuite) TestChangeInvalidID(c *check.C) {
+	_, err := cs.cli.Change("select * from users;")
+	c.Assert(err, check.ErrorMatches, "invalid change ID.*")
+}
+
+func (cs *clientSuite) TestAbortInvalidID(c *check.C) {
+	_, err := cs.cli.Abort("<foo>")
+	c.Assert(err, check.ErrorMatches, "invalid change ID.*")
+}
+
+func (cs *clientSuite) TestWaitChangeInvalidID(c *check.C) {
+	_, err := cs.cli.WaitChange("$bar", nil)
+	c.Assert(err, check.ErrorMatches, "invalid change ID.*")
+}