6222: Precompile all regexps. Remove wasted effort in GetBlockHandler.
authorTom Clegg <tom@curoverse.com>
Sat, 6 Jun 2015 20:29:48 +0000 (16:29 -0400)
committerTom Clegg <tom@curoverse.com>
Tue, 16 Jun 2015 20:08:56 +0000 (16:08 -0400)
services/keepstore/handler_test.go
services/keepstore/handlers.go
services/keepstore/perms.go
services/keepstore/perms_test.go

index 5bd14be3499e123c648a6f97749a911d77ae5dd8..074e49999b449eca9dd82ba418fa5f0fe9401e8e 100644 (file)
@@ -195,7 +195,7 @@ func TestPutHandler(t *testing.T) {
                "Authenticated PUT, signed locator, with server key",
                http.StatusOK, response)
        response_locator := strings.TrimSpace(response.Body.String())
-       if !VerifySignature(response_locator, known_token) {
+       if VerifySignature(response_locator, known_token) != nil {
                t.Errorf("Authenticated PUT, signed locator, with server key:\n"+
                        "response '%s' does not contain a valid signature",
                        response_locator)
@@ -788,7 +788,7 @@ func ExpectStatusCode(
        expected_status int,
        response *httptest.ResponseRecorder) {
        if response.Code != expected_status {
-               t.Errorf("%s: expected status %s, got %+v",
+               t.Errorf("%s: expected status %d, got %+v",
                        testname, expected_status, response)
        }
 }
index 8930b79c338883327b404c2a39375f54cdf3dec3..2b437e75cb8b4a13ea64ed9fca1f6d78f0fffb66 100644 (file)
@@ -20,7 +20,6 @@ import (
        "os"
        "regexp"
        "strconv"
-       "strings"
        "syscall"
        "time"
 )
@@ -66,51 +65,15 @@ func BadRequestHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func GetBlockHandler(resp http.ResponseWriter, req *http.Request) {
-       hash := mux.Vars(req)["hash"]
-
-       hints := mux.Vars(req)["hints"]
-
-       // Parse the locator string and hints from the request.
-       // TODO(twp): implement a Locator type.
-       var signature, timestamp string
-       if hints != "" {
-               signature_pat, _ := regexp.Compile("^A([[:xdigit:]]+)@([[:xdigit:]]{8})$")
-               for _, hint := range strings.Split(hints, "+") {
-                       if match, _ := regexp.MatchString("^[[:digit:]]+$", hint); match {
-                               // Server ignores size hints
-                       } else if m := signature_pat.FindStringSubmatch(hint); m != nil {
-                               signature = m[1]
-                               timestamp = m[2]
-                       } else if match, _ := regexp.MatchString("^[[:upper:]]", hint); match {
-                               // Any unknown hint that starts with an uppercase letter is
-                               // presumed to be valid and ignored, to permit forward compatibility.
-                       } else {
-                               // Unknown format; not a valid locator.
-                               http.Error(resp, BadRequestError.Error(), BadRequestError.HTTPCode)
-                               return
-                       }
-               }
-       }
-
-       // If permission checking is in effect, verify this
-       // request's permission signature.
        if enforce_permissions {
-               if signature == "" || timestamp == "" {
-                       http.Error(resp, PermissionError.Error(), PermissionError.HTTPCode)
-                       return
-               } else if IsExpired(timestamp) {
-                       http.Error(resp, ExpiredError.Error(), ExpiredError.HTTPCode)
+               locator := req.URL.Path[1:] // strip leading slash
+               if err := VerifySignature(locator, GetApiToken(req)); err != nil {
+                       http.Error(resp, err.Error(), err.(*KeepError).HTTPCode)
                        return
-               } else {
-                       req_locator := req.URL.Path[1:] // strip leading slash
-                       if !VerifySignature(req_locator, GetApiToken(req)) {
-                               http.Error(resp, PermissionError.Error(), PermissionError.HTTPCode)
-                               return
-                       }
                }
        }
 
-       block, err := GetBlock(hash, false)
+       block, err := GetBlock(mux.Vars(req)["hash"], false)
        if err != nil {
                // This type assertion is safe because the only errors
                // GetBlock can return are DiskHashError or NotFoundError.
@@ -630,28 +593,25 @@ func PutBlock(block []byte, hash string) error {
        }
 }
 
+var validLocatorRe = regexp.MustCompile(`^[0-9a-f]{32}$`)
+
 // IsValidLocator
 //     Return true if the specified string is a valid Keep locator.
 //     When Keep is extended to support hash types other than MD5,
 //     this should be updated to cover those as well.
 //
 func IsValidLocator(loc string) bool {
-       match, err := regexp.MatchString(`^[0-9a-f]{32}$`, loc)
-       if err == nil {
-               return match
-       }
-       log.Printf("IsValidLocator: %s\n", err)
-       return false
+       return validLocatorRe.MatchString(loc)
 }
 
+var authRe = regexp.MustCompile(`^OAuth2\s+(.*)`)
+
 // GetApiToken returns the OAuth2 token from the Authorization
 // header of a HTTP request, or an empty string if no matching
 // token is found.
 func GetApiToken(req *http.Request) string {
        if auth, ok := req.Header["Authorization"]; ok {
-               if pat, err := regexp.Compile(`^OAuth2\s+(.*)`); err != nil {
-                       log.Println(err)
-               } else if match := pat.FindStringSubmatch(auth[0]); match != nil {
+               if match := authRe.FindStringSubmatch(auth[0]); match != nil {
                        return match[1]
                }
        }
index 1048f53130315525da0a80c1214207a9eb101cd5..65160b1868913638e8315a266e0b3736ecfbe14c 100644 (file)
@@ -82,22 +82,29 @@ func SignLocator(blob_locator string, api_token string, expiry time.Time) string
 
 var signedLocatorRe = regexp.MustCompile(`^([[:xdigit:]]{32}).*\+A([[:xdigit:]]{40})@([[:xdigit:]]{8})`)
 
-// VerifySignature returns true if the signature on the signed_locator
-// can be verified using the given api_token.
-func VerifySignature(signed_locator string, api_token string) bool {
+// VerifySignature returns nil if the signature on the signed_locator
+// can be verified using the given api_token. Otherwise it returns
+// either ExpiredError (if the timestamp has expired, which is
+// something the client could have figured out independently) or
+// PermissionError.
+func VerifySignature(signed_locator string, api_token string) error {
        matches := signedLocatorRe.FindStringSubmatch(signed_locator)
        if matches == nil {
                // Could not find a permission signature at all
-               return false
+               return PermissionError
        }
        blob_hash := matches[1]
        sig_hex := matches[2]
        exp_hex := matches[3]
-       if exp_time, err := ParseHexTimestamp(exp_hex); err != nil || exp_time.Before(time.Now()) {
-               // Signature is expired, or timestamp is unparseable
-               return false
+       if exp_time, err := ParseHexTimestamp(exp_hex); err != nil {
+               return PermissionError
+       } else if exp_time.Before(time.Now()) {
+               return ExpiredError
        }
-       return sig_hex == MakePermSignature(blob_hash, api_token, exp_hex)
+       if sig_hex != MakePermSignature(blob_hash, api_token, exp_hex) {
+               return PermissionError
+       }
+       return nil
 }
 
 func ParseHexTimestamp(timestamp_hex string) (ts time.Time, err error) {
index 7367dbf0adc33d47b74e34fda37d898c4e34242d..85883b03a7d33a416947d823957f1ec1a3ffac7d 100644 (file)
@@ -39,7 +39,7 @@ func TestVerifySignature(t *testing.T) {
        PermissionSecret = []byte(known_key)
        defer func() { PermissionSecret = nil }()
 
-       if !VerifySignature(known_signed_locator, known_token) {
+       if VerifySignature(known_signed_locator, known_token) != nil {
                t.Fail()
        }
 }
@@ -48,15 +48,15 @@ func TestVerifySignatureExtraHints(t *testing.T) {
        PermissionSecret = []byte(known_key)
        defer func() { PermissionSecret = nil }()
 
-       if !VerifySignature(known_locator+"+K@xyzzy"+known_sig_hint, known_token) {
+       if VerifySignature(known_locator+"+K@xyzzy"+known_sig_hint, known_token) != nil{
                t.Fatal("Verify cannot handle hint before permission signature")
        }
 
-       if !VerifySignature(known_locator+known_sig_hint+"+Zfoo", known_token) {
+       if VerifySignature(known_locator+known_sig_hint+"+Zfoo", known_token) != nil {
                t.Fatal("Verify cannot handle hint after permission signature")
        }
 
-       if !VerifySignature(known_locator+"+K@xyzzy"+known_sig_hint+"+Zfoo", known_token) {
+       if VerifySignature(known_locator+"+K@xyzzy"+known_sig_hint+"+Zfoo", known_token) != nil {
                t.Fatal("Verify cannot handle hints around permission signature")
        }
 }
@@ -66,11 +66,11 @@ func TestVerifySignatureWrongSize(t *testing.T) {
        PermissionSecret = []byte(known_key)
        defer func() { PermissionSecret = nil }()
 
-       if !VerifySignature(known_hash+"+999999"+known_sig_hint, known_token) {
+       if VerifySignature(known_hash+"+999999"+known_sig_hint, known_token) != nil {
                t.Fatal("Verify cannot handle incorrect size hint")
        }
 
-       if !VerifySignature(known_hash+known_sig_hint, known_token) {
+       if VerifySignature(known_hash+known_sig_hint, known_token) != nil {
                t.Fatal("Verify cannot handle missing size hint")
        }
 }
@@ -80,7 +80,7 @@ func TestVerifySignatureBadSig(t *testing.T) {
        defer func() { PermissionSecret = nil }()
 
        bad_locator := known_locator + "+Aaaaaaaaaaaaaaaa@" + known_timestamp
-       if VerifySignature(bad_locator, known_token) {
+       if VerifySignature(bad_locator, known_token) != PermissionError {
                t.Fail()
        }
 }
@@ -89,8 +89,8 @@ func TestVerifySignatureBadTimestamp(t *testing.T) {
        PermissionSecret = []byte(known_key)
        defer func() { PermissionSecret = nil }()
 
-       bad_locator := known_locator + "+A" + known_signature + "@00000000"
-       if VerifySignature(bad_locator, known_token) {
+       bad_locator := known_locator + "+A" + known_signature + "@OOOOOOOl"
+       if VerifySignature(bad_locator, known_token) != PermissionError {
                t.Fail()
        }
 }
@@ -99,7 +99,7 @@ func TestVerifySignatureBadSecret(t *testing.T) {
        PermissionSecret = []byte("00000000000000000000")
        defer func() { PermissionSecret = nil }()
 
-       if VerifySignature(known_signed_locator, known_token) {
+       if VerifySignature(known_signed_locator, known_token) != PermissionError {
                t.Fail()
        }
 }
@@ -108,7 +108,7 @@ func TestVerifySignatureBadToken(t *testing.T) {
        PermissionSecret = []byte(known_key)
        defer func() { PermissionSecret = nil }()
 
-       if VerifySignature(known_signed_locator, "00000000") {
+       if VerifySignature(known_signed_locator, "00000000") != PermissionError {
                t.Fail()
        }
 }
@@ -119,7 +119,7 @@ func TestVerifySignatureExpired(t *testing.T) {
 
        yesterday := time.Now().AddDate(0, 0, -1)
        expired_locator := SignLocator(known_hash, known_token, yesterday)
-       if VerifySignature(expired_locator, known_token) {
+       if VerifySignature(expired_locator, known_token) != ExpiredError {
                t.Fail()
        }
 }