From 6b028afce4f8c0e7e39e7a4a89771fa3c26f17af Mon Sep 17 00:00:00 2001 From: Yuki Yugui Sonoda Date: Sun, 30 Aug 2015 11:56:46 +0900 Subject: [PATCH 1/2] Precalculate tail length of pattern Tail length means the length of fixed-size segements after a deep wildcard in the pattern. This helps us to care of #23 on pattern matching. --- runtime/pattern.go | 28 +++++++++++++++++++---- runtime/pattern_test.go | 50 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/runtime/pattern.go b/runtime/pattern.go index 694231ff9d7..ca82d2dd9a5 100644 --- a/runtime/pattern.go +++ b/runtime/pattern.go @@ -31,6 +31,8 @@ type Pattern struct { vars []string // stacksize is the max depth of the stack stacksize int + // tailLen is the length of the fixed-size segments after a deep wildcard + tailLen int // verb is the VERB part of the path pattern. It is empty if the pattern does not have VERB part. verb string } @@ -52,21 +54,38 @@ func NewPattern(version int, ops []int, pool []string, verb string) (Pattern, er return Pattern{}, ErrInvalidPattern } - var typedOps []op - var stack, maxstack int - var vars []string + var ( + typedOps []op + stack, maxstack int + tailLen int + pushMSeen bool + vars []string + ) for i := 0; i < l; i += 2 { op := op{code: utilities.OpCode(ops[i]), operand: ops[i+1]} switch op.code { case utilities.OpNop: continue - case utilities.OpPush, utilities.OpPushM: + case utilities.OpPush: + if pushMSeen { + tailLen++ + } + stack++ + case utilities.OpPushM: + if pushMSeen { + glog.V(2).Info("pushM appears twice") + return Pattern{}, ErrInvalidPattern + } + pushMSeen = true stack++ case utilities.OpLitPush: if op.operand < 0 || len(pool) <= op.operand { glog.V(2).Infof("negative literal index: %d", op.operand) return Pattern{}, ErrInvalidPattern } + if pushMSeen { + tailLen++ + } stack++ case utilities.OpConcatN: if op.operand <= 0 { @@ -108,6 +127,7 @@ func NewPattern(version int, ops []int, pool []string, verb string) (Pattern, er pool: pool, vars: vars, stacksize: maxstack, + tailLen: tailLen, verb: verb, }, nil } diff --git a/runtime/pattern_test.go b/runtime/pattern_test.go index d81916539a5..5da6669d228 100644 --- a/runtime/pattern_test.go +++ b/runtime/pattern_test.go @@ -20,25 +20,29 @@ func TestNewPattern(t *testing.T) { pool []string verb string - stackSizeWant int + stackSizeWant, tailLenWant int }{ {}, { ops: []int{int(utilities.OpNop), anything}, stackSizeWant: 0, + tailLenWant: 0, }, { ops: []int{int(utilities.OpPush), anything}, stackSizeWant: 1, + tailLenWant: 0, }, { ops: []int{int(utilities.OpLitPush), 0}, pool: []string{"abc"}, stackSizeWant: 1, + tailLenWant: 0, }, { ops: []int{int(utilities.OpPushM), anything}, stackSizeWant: 1, + tailLenWant: 0, }, { ops: []int{ @@ -46,6 +50,7 @@ func TestNewPattern(t *testing.T) { int(utilities.OpConcatN), 1, }, stackSizeWant: 1, + tailLenWant: 0, }, { ops: []int{ @@ -55,6 +60,7 @@ func TestNewPattern(t *testing.T) { }, pool: []string{"abc"}, stackSizeWant: 1, + tailLenWant: 0, }, { ops: []int{ @@ -67,12 +73,40 @@ func TestNewPattern(t *testing.T) { }, pool: []string{"lit1", "lit2", "var1"}, stackSizeWant: 4, + tailLenWant: 0, + }, + { + ops: []int{ + int(utilities.OpPushM), anything, + int(utilities.OpConcatN), 1, + int(utilities.OpCapture), 2, + int(utilities.OpLitPush), 0, + int(utilities.OpLitPush), 1, + }, + pool: []string{"lit1", "lit2", "var1"}, + stackSizeWant: 2, + tailLenWant: 2, + }, + { + ops: []int{ + int(utilities.OpLitPush), 0, + int(utilities.OpLitPush), 1, + int(utilities.OpPushM), anything, + int(utilities.OpLitPush), 2, + int(utilities.OpConcatN), 3, + int(utilities.OpLitPush), 3, + int(utilities.OpCapture), 4, + }, + pool: []string{"lit1", "lit2", "lit3", "lit4", "var1"}, + stackSizeWant: 4, + tailLenWant: 2, }, { ops: []int{int(utilities.OpLitPush), 0}, pool: []string{"abc"}, - stackSizeWant: 1, verb: "LOCK", + stackSizeWant: 1, + tailLenWant: 0, }, } { pat, err := NewPattern(validVersion, spec.ops, spec.pool, spec.verb) @@ -83,6 +117,9 @@ func TestNewPattern(t *testing.T) { if got, want := pat.stacksize, spec.stackSizeWant; got != want { t.Errorf("pat.stacksize = %d; want %d", got, want) } + if got, want := pat.tailLen, spec.tailLenWant; got != want { + t.Errorf("pat.stacksize = %d; want %d", got, want) + } } } @@ -129,6 +166,15 @@ func TestNewPatternWithWrongOp(t *testing.T) { ops: []int{int(utilities.OpCapture), 1}, pool: []string{"abc"}, }, + { + // pushM appears twice + ops: []int{ + int(utilities.OpPushM), anything, + int(utilities.OpLitPush), 0, + int(utilities.OpPushM), anything, + }, + pool: []string{"abc"}, + }, } { _, err := NewPattern(validVersion, spec.ops, spec.pool, spec.verb) if err == nil { From 960d05fc956e22c15c1526a99ea4622932bab111 Mon Sep 17 00:00:00 2001 From: Yuki Yugui Sonoda Date: Sun, 30 Aug 2015 12:17:54 +0900 Subject: [PATCH 2/2] Consider tail after deep wildcard on matching Fixes #23 --- runtime/pattern.go | 9 +++++++-- runtime/pattern_test.go | 38 ++++++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/runtime/pattern.go b/runtime/pattern.go index ca82d2dd9a5..77931d56537 100644 --- a/runtime/pattern.go +++ b/runtime/pattern.go @@ -173,8 +173,13 @@ func (p Pattern) Match(components []string, verb string) (map[string]string, err stack = append(stack, c) pos++ case utilities.OpPushM: - stack = append(stack, strings.Join(components[pos:], "/")) - pos = len(components) + end := len(components) + if end < pos+p.tailLen { + return nil, ErrNotMatch + } + end -= p.tailLen + stack = append(stack, strings.Join(components[pos:end], "/")) + pos = end case utilities.OpConcatN: n := op.operand l := len(stack) - n diff --git a/runtime/pattern_test.go b/runtime/pattern_test.go index 5da6669d228..55d6cb8b161 100644 --- a/runtime/pattern_test.go +++ b/runtime/pattern_test.go @@ -247,6 +247,18 @@ func TestMatch(t *testing.T) { ops: []int{int(utilities.OpPushM), anything}, match: []string{"", "abc", "abc/def", "abc/def/ghi"}, }, + { + ops: []int{ + int(utilities.OpPushM), anything, + int(utilities.OpLitPush), 0, + }, + pool: []string{"tail"}, + match: []string{"tail", "abc/tail", "abc/def/tail"}, + notMatch: []string{ + "", "abc", "abc/def", + "tail/extra", "abc/tail/extra", "abc/def/tail/extra", + }, + }, { ops: []int{ int(utilities.OpLitPush), 0, @@ -430,6 +442,22 @@ func TestMatchWithBinding(t *testing.T) { "name": "o/my-bucket/dir/dir2/obj", }, }, + { + ops: []int{ + int(utilities.OpLitPush), 0, + int(utilities.OpLitPush), 1, + int(utilities.OpPushM), anything, + int(utilities.OpLitPush), 2, + int(utilities.OpConcatN), 3, + int(utilities.OpCapture), 4, + int(utilities.OpLitPush), 3, + }, + pool: []string{"v1", "o", ".ext", "tail", "name"}, + path: "v1/o/my-bucket/dir/dir2/obj/.ext/tail", + want: map[string]string{ + "name": "o/my-bucket/dir/dir2/obj/.ext", + }, + }, { ops: []int{ int(utilities.OpLitPush), 0, @@ -531,11 +559,13 @@ func TestPatternString(t *testing.T) { int(utilities.OpCapture), 2, int(utilities.OpLitPush), 3, int(utilities.OpPushM), anything, - int(utilities.OpConcatN), 2, - int(utilities.OpCapture), 4, + int(utilities.OpLitPush), 4, + int(utilities.OpConcatN), 3, + int(utilities.OpCapture), 6, + int(utilities.OpLitPush), 5, }, - pool: []string{"v1", "buckets", "bucket_name", "objects", "name"}, - want: "/v1/{bucket_name=buckets/*}/{name=objects/**}", + pool: []string{"v1", "buckets", "bucket_name", "objects", ".ext", "tail", "name"}, + want: "/v1/{bucket_name=buckets/*}/{name=objects/**/.ext}/tail", }, } { p, err := NewPattern(validVersion, spec.ops, spec.pool, "")