Skip to content

Commit

Permalink
Merge pull request moby#48169 from kolyshkin/layer-regexp
Browse files Browse the repository at this point in the history
rm regexp use
  • Loading branch information
vvoland authored Jul 17, 2024
2 parents aae0440 + 5089398 commit 3a62d49
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 23 deletions.
27 changes: 20 additions & 7 deletions daemon/containerd/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ import (
"golang.org/x/sync/semaphore"
)

var truncatedID = regexp.MustCompile(`^(sha256:)?([a-f0-9]{4,64})$`)

var errInconsistentData error = errors.New("consistency error: data changed during operation, retry")

// GetImage returns an image corresponding to the image referred to by refOrID.
Expand Down Expand Up @@ -326,9 +324,8 @@ func (i *ImageService) resolveImage(ctx context.Context, refOrID string) (contai
}
}

// If the identifier could be a short ID, attempt to match
if truncatedID.MatchString(refOrID) {
idWithoutAlgo := strings.TrimPrefix(refOrID, "sha256:")
// If the identifier could be a short ID, attempt to match.
if idWithoutAlgo := checkTruncatedID(refOrID); idWithoutAlgo != "" { // Valid ID.
filters := []string{
fmt.Sprintf("name==%q", ref), // Or it could just look like one.
"target.digest~=" + strconv.Quote(fmt.Sprintf(`^sha256:%s[0-9a-fA-F]{%d}$`, regexp.QuoteMeta(idWithoutAlgo), 64-len(idWithoutAlgo))),
Expand Down Expand Up @@ -435,7 +432,7 @@ func (i *ImageService) resolveAllReferences(ctx context.Context, refOrID string)
var dgst digest.Digest
var img *containerdimages.Image

if truncatedID.MatchString(refOrID) {
if idWithoutAlgo := checkTruncatedID(refOrID); idWithoutAlgo != "" { // Valid ID.
if d, ok := parsed.(reference.Digested); ok {
if cimg, err := i.images.Get(ctx, d.String()); err == nil {
img = &cimg
Expand All @@ -451,7 +448,6 @@ func (i *ImageService) resolveAllReferences(ctx context.Context, refOrID string)
dgst = d.Digest()
}
} else {
idWithoutAlgo := strings.TrimPrefix(refOrID, "sha256:")
name := reference.TagNameOnly(parsed.(reference.Named)).String()
filters := []string{
fmt.Sprintf("name==%q", name), // Or it could just look like one.
Expand Down Expand Up @@ -551,3 +547,20 @@ func (i *ImageService) resolveAllReferences(ctx context.Context, refOrID string)

return img, imgs, nil
}

// checkTruncatedID checks id for validity. If id is invalid, an empty string
// is returned; otherwise, the ID without the optional "sha256:" prefix is
// returned. The validity check is equivalent to
// regexp.MustCompile(`^(sha256:)?([a-f0-9]{4,64})$`).MatchString(id).
func checkTruncatedID(id string) string {
id = strings.TrimPrefix(id, "sha256:")
if l := len(id); l < 4 || l > 64 {
return ""
}
for _, c := range id {
if (c < '0' || c > '9') && (c < 'a' || c > 'f') {
return ""
}
}
return id
}
9 changes: 4 additions & 5 deletions image/v1/imagev1.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"encoding/json"
"errors"
"regexp"
"strings"

"github.com/containerd/log"
Expand All @@ -23,8 +22,6 @@ const (
fullLen = 64
)

var validHex = regexp.MustCompile(`^[a-f0-9]{64}$`)

// HistoryFromConfig creates a History struct from v1 configuration JSON
func HistoryFromConfig(imageJSON []byte, emptyLayer bool) (image.History, error) {
h := image.History{}
Expand Down Expand Up @@ -126,8 +123,10 @@ func ValidateID(id string) error {
if len(id) != fullLen {
return errors.New("image ID '" + id + "' is invalid")
}
if !validHex.MatchString(id) {
return errors.New("image ID '" + id + "' is invalid")
for _, c := range id {
if (c < '0' || c > '9') && (c < 'a' || c > 'f') {
return errors.New("image ID '" + id + "' is invalid")
}
}
return nil
}
33 changes: 22 additions & 11 deletions layer/filestore.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"io"
"os"
"path/filepath"
"regexp"
"strconv"
"strings"

Expand All @@ -18,14 +17,11 @@ import (
"github.com/pkg/errors"
)

var (
stringIDRegexp = regexp.MustCompile(`^[a-f0-9]{64}(-init)?$`)
supportedAlgorithms = []digest.Algorithm{
digest.SHA256,
// digest.SHA384, // Currently not used
// digest.SHA512, // Currently not used
}
)
var supportedAlgorithms = []digest.Algorithm{
digest.SHA256,
// digest.SHA384, // Currently not used
// digest.SHA512, // Currently not used
}

type fileMetadataStore struct {
root string
Expand Down Expand Up @@ -262,7 +258,7 @@ func (fms *fileMetadataStore) GetMountID(mount string) (string, error) {
}
content := strings.TrimSpace(string(contentBytes))

if !stringIDRegexp.MatchString(content) {
if !isValidID(content) {
return "", errors.New("invalid mount id value")
}

Expand All @@ -279,7 +275,7 @@ func (fms *fileMetadataStore) GetInitID(mount string) (string, error) {
}
content := strings.TrimSpace(string(contentBytes))

if !stringIDRegexp.MatchString(content) {
if !isValidID(content) {
return "", errors.New("invalid init id value")
}

Expand Down Expand Up @@ -431,3 +427,18 @@ func (fms *fileMetadataStore) Remove(layer ChainID, cache string) error {
func (fms *fileMetadataStore) RemoveMount(mount string) error {
return os.RemoveAll(fms.getMountDirectory(mount))
}

// isValidID checks if mount/init id is valid. It is similar to
// regexp.MustCompile(`^[a-f0-9]{64}(-init)?$`).MatchString(id).
func isValidID(id string) bool {
id = strings.TrimSuffix(id, "-init")
if len(id) != 64 {
return false
}
for _, c := range id {
if (c < '0' || c > '9') && (c < 'a' || c > 'f') {
return false
}
}
return true
}
26 changes: 26 additions & 0 deletions layer/filestore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,29 @@ func TestGetOrphan(t *testing.T) {
t.Fatalf("Expected to have one orphan layer")
}
}

func TestIsValidID(t *testing.T) {
testCases := []struct {
name string
id string
expected bool
}{
{"Valid 64-char hexadecimal", "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", true},
{"Valid 64-char hexadecimal with -init suffix", "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef-init", true},
{"Invalid: too short", "1234567890abcdef", false},
{"Invalid: too long", "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef00", false},
{"Invalid: contains uppercase letter", "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdeF", false},
{"Invalid: contains non-hexadecimal character", "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdeg", false},
{"Invalid: empty string", "", false},
{"Invalid: only -init suffix", "-init", false},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
result := isValidID(tc.id)
if result != tc.expected {
t.Errorf("isValidID(%q): got %v, want %v", tc.id, result, tc.expected)
}
})
}
}

0 comments on commit 3a62d49

Please sign in to comment.