Skip to content

Commit

Permalink
Fix context bug which could hang forever (#960)
Browse files Browse the repository at this point in the history
We could hang forever if we have irregular file types present, like
pipes which never close.

Here we fix the issue by only hashing regular files.

Fixes #614
  • Loading branch information
blampe committed Jan 26, 2024
1 parent 1d6e638 commit ea3bb3e
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 3 deletions.
2 changes: 1 addition & 1 deletion provider/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,7 @@ func processLogLine(jm jsonmessage.JSONMessage,
info += "failed to parse aux message: " + err.Error()
}
if err := (&resp).Unmarshal(infoBytes); err != nil {
info += "failed to parse aux message: " + err.Error()
info += "failed to parse info bytes: " + err.Error()
}
for _, vertex := range resp.Vertexes {
info += fmt.Sprintf("digest: %+v\n", vertex.Digest)
Expand Down
4 changes: 2 additions & 2 deletions provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ func (accumulator *contextHashAccumulator) hashPath(
if err != nil {
return fmt.Errorf("could not copy symlink path %s to hash: %w", filePath, err)
}
} else {
} else if fileMode.IsRegular() {
// For regular files, we can hash their content.
// TODO: consider only hashing file metadata to improve performance
f, err := os.Open(filePath)
Expand Down Expand Up @@ -737,7 +737,7 @@ func setConfiguration(configVars map[string]string) map[string]string {
}

func marshalBuildOnPreview(inputs resource.PropertyMap) bool {
//set default if not set
// set default if not set
if inputs["buildOnPreview"].IsNull() || inputs["buildOnPreview"].ContainsUnknowns() {
return false
}
Expand Down
23 changes: 23 additions & 0 deletions provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"os"
"path/filepath"
"syscall"
"testing"

"github.com/docker/distribution/reference"
Expand Down Expand Up @@ -238,6 +239,28 @@ func TestHashDeepSymlinks(t *testing.T) {
assert.NoError(t, err)
}

func TestIgnoreIrregularFiles(t *testing.T) {
dir := t.TempDir()

// Create a Dockerfile
dockerfile := filepath.Join(dir, "Dockerfile")
err := os.WriteFile(dockerfile, []byte{}, 0o600)
require.NoError(t, err)

// Create a pipe which should be ignored. (We will time out trying to read
// it if it's not.)
pipe := filepath.Join(dir, "pipe")
err = syscall.Mkfifo(pipe, 0o666)
require.NoError(t, err)
// Confirm it's irregular.
fi, err := os.Stat(pipe)
require.NoError(t, err)
assert.False(t, fi.Mode().IsRegular())

_, err = hashContext(dir, dockerfile)
assert.NoError(t, err)
}

func TestHashUnignoredDirs(t *testing.T) {
step1Dir := "./testdata/unignores/basedir"
baseResult, err := hashContext(step1Dir, filepath.Join(step1Dir, defaultDockerfile))
Expand Down

0 comments on commit ea3bb3e

Please sign in to comment.