Merge pull request #157 from Pix4D/fix_regexp_anchors_regression

Fix regexp anchors regression
concourse · Mar 16, 2022 · 47dc586 · 47dc586
2 parents 5d50637 + 8b41fc2
commit 47dc586
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 2 deletions.
diff --git a/README.md b/README.md
@@ -66,6 +66,9 @@ One of the following two options must be specified:
   Semantic versions, or just numbers, are supported. Accordingly, full regular
   expressions are supported, to specify the capture groups.
 
+  The full `regexp` will be matched against the S3 objects as if it was anchored
+  on both ends, even if you don't specify `^` and `$` explicitly.
+
 * `versioned_file`: *Optional* If you enable versioning for your S3 bucket then
   you can keep the file name the same and upload new versions of your file
   without resorting to version numbers. This property is the path to the file

diff --git a/versions/versions.go b/versions/versions.go
@@ -115,6 +115,13 @@ func GetMatchingPathsFromBucket(client s3resource.S3Client, bucketName string, r
 
 	specialCharsRE := regexp.MustCompile(`[\\\*\.\[\]\(\)\{\}\?\|\^\$\+]`)
 
+	if strings.HasPrefix(regex, "^") {
+		regex = regex[1:]
+	}
+	if strings.HasSuffix(regex, "$") {
+		regex = regex[:len(regex)-1]
+	}
+
 	matchingPaths := []string{}
 	queue := []work{{prefix: "", remains: strings.Split(regex, "/")}}
 	for len(queue) != 0 {
@@ -134,9 +141,9 @@ func GetMatchingPathsFromBucket(client s3resource.S3Client, bucketName string, r
 		var prefixRE *regexp.Regexp
 		if len(remains) != 0 {
 			// We need to look deeper so full prefix will end with a /
-			prefixRE = regexp.MustCompile(prefix + section + "/")
+			prefixRE = regexp.MustCompile("^" + prefix + section + "/$")
 		} else {
-			prefixRE = regexp.MustCompile(prefix + section)
+			prefixRE = regexp.MustCompile("^" + prefix + section + "$")
 		}
 		var (
 			continuationToken *string

diff --git a/versions/versions_test.go b/versions/versions_test.go
@@ -293,6 +293,86 @@ var _ = Describe("GetMatchingPathsFromBucket", func() {
 		})
 	})
 
+	Context("When regexp is not anchored explicitly and has no prefix", func() {
+		It("will behave as if anchored at both ends", func() {
+			s3client.ChunkedBucketListReturnsOnCall(0, s3resource.BucketListChunk{
+				Paths: []string{
+					"substring",
+					"also-substring",
+					"subscribing",
+					"substring.suffix",
+				},
+			}, nil)
+
+			matchingPaths, err := versions.GetMatchingPathsFromBucket(
+				s3client, "bucket", "sub(.*)ing",
+			)
+			Ω(err).ShouldNot(HaveOccurred())
+			Ω(s3client.ChunkedBucketListCallCount()).Should(Equal(1))
+			Ω(matchingPaths).Should(ConsistOf("substring", "subscribing"))
+		})
+	})
+
+	Context("When regexp is not anchored explicitly and has prefix", func() {
+		It("will behave as if anchored at both ends", func() {
+			s3client.ChunkedBucketListReturnsOnCall(0, s3resource.BucketListChunk{
+				Paths: []string{
+					"pre/ssing",
+					"pre/singer",
+				},
+			}, nil)
+
+			matchingPaths, err := versions.GetMatchingPathsFromBucket(
+				s3client, "bucket", "pre/(.*)ing",
+			)
+			Ω(err).ShouldNot(HaveOccurred())
+			Ω(s3client.ChunkedBucketListCallCount()).Should(Equal(1))
+			_, prefix, _ := s3client.ChunkedBucketListArgsForCall(0)
+			Ω(prefix).Should(Equal("pre/"))
+			Ω(matchingPaths).Should(ConsistOf("pre/ssing"))
+		})
+	})
+
+	Context("When regexp is anchored explicitly and has not prefix", func() {
+		It("still works", func() {
+			s3client.ChunkedBucketListReturnsOnCall(0, s3resource.BucketListChunk{
+				Paths: []string{
+					"substring",
+					"also-substring",
+					"subscribing",
+					"substring.suffix",
+				},
+			}, nil)
+
+			matchingPaths, err := versions.GetMatchingPathsFromBucket(
+				s3client, "bucket", "^sub(.*)ing$",
+			)
+			Ω(err).ShouldNot(HaveOccurred())
+			Ω(s3client.ChunkedBucketListCallCount()).Should(Equal(1))
+			Ω(matchingPaths).Should(ConsistOf("substring", "subscribing"))
+		})
+	})
+
+	Context("When regexp is anchored explicitly and has prefix", func() {
+		It("still works", func() {
+			s3client.ChunkedBucketListReturnsOnCall(0, s3resource.BucketListChunk{
+				Paths: []string{
+					"pre/ssing",
+					"pre/singer",
+				},
+			}, nil)
+
+			matchingPaths, err := versions.GetMatchingPathsFromBucket(
+				s3client, "bucket", "^pre/(.*)ing$",
+			)
+			Ω(err).ShouldNot(HaveOccurred())
+			Ω(s3client.ChunkedBucketListCallCount()).Should(Equal(1))
+			_, prefix, _ := s3client.ChunkedBucketListArgsForCall(0)
+			Ω(prefix).Should(Equal("pre/"))
+			Ω(matchingPaths).Should(ConsistOf("pre/ssing"))
+		})
+	})
+
 	Context("When S3 returns an error", func() {
 		BeforeEach(func() {
 			s3client.ChunkedBucketListReturns(