Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid panic on temp file deletion #988

Merged
merged 18 commits into from
Aug 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions artifactory/services/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"errors"
"fmt"
"github.com/jfrog/build-info-go/entities"
biutils "github.com/jfrog/build-info-go/utils"
"github.com/jfrog/gofrog/crypto"
ioutils "github.com/jfrog/gofrog/io"
"github.com/jfrog/gofrog/version"
"net/http"
Expand Down Expand Up @@ -495,11 +495,11 @@ func createLocalSymlink(localPath, localFileName, symlinkArtifact string, symlin
if !fileutils.IsPathExists(symlinkArtifact, false) {
return errorutils.CheckErrorf("symlink validation failed, target doesn't exist: " + symlinkArtifact)
}
var checksums map[biutils.Algorithm]string
if checksums, err = biutils.GetFileChecksums(symlinkArtifact, biutils.SHA1); err != nil {
var checksums map[crypto.Algorithm]string
if checksums, err = crypto.GetFileChecksums(symlinkArtifact, crypto.SHA1); err != nil {
return errorutils.CheckError(err)
}
if checksums[biutils.SHA1] != symlinkContentChecksum {
if checksums[crypto.SHA1] != symlinkContentChecksum {
return errorutils.CheckErrorf("symlink validation failed for target: " + symlinkArtifact)
}
}
Expand Down
10 changes: 5 additions & 5 deletions artifactory/services/fspatterns/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ package fspatterns
import (
"bytes"
"fmt"
"github.com/jfrog/gofrog/crypto"
"github.com/jfrog/jfrog-client-go/utils/log"
"os"
"regexp"
"strings"

biutils "github.com/jfrog/build-info-go/utils"
"github.com/jfrog/jfrog-client-go/utils"
"github.com/jfrog/jfrog-client-go/utils/errorutils"
"github.com/jfrog/jfrog-client-go/utils/io/fileutils"
Expand Down Expand Up @@ -192,15 +192,15 @@ func GetRootPath(pattern, target, archiveTarget string, patternType utils.Patter

// When handling symlink we want to simulate the creation of empty file
func CreateSymlinkFileDetails() (*fileutils.FileDetails, error) {
checksums, err := biutils.CalcChecksums(bytes.NewBuffer([]byte(fileutils.SymlinkFileContent)))
checksums, err := crypto.CalcChecksums(bytes.NewBuffer([]byte(fileutils.SymlinkFileContent)))
if err != nil {
return nil, errorutils.CheckError(err)
}

details := new(fileutils.FileDetails)
details.Checksum.Md5 = checksums[biutils.MD5]
details.Checksum.Sha1 = checksums[biutils.SHA1]
details.Checksum.Sha256 = checksums[biutils.SHA256]
details.Checksum.Md5 = checksums[crypto.MD5]
details.Checksum.Sha1 = checksums[crypto.SHA1]
details.Checksum.Sha256 = checksums[crypto.SHA256]
details.Size = int64(0)
return details, nil
}
8 changes: 4 additions & 4 deletions artifactory/services/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"archive/zip"
"errors"
"fmt"
"github.com/jfrog/gofrog/crypto"
"io"
"net/http"
"os"
Expand All @@ -14,7 +15,6 @@ import (
"sync"

"github.com/jfrog/build-info-go/entities"
biutils "github.com/jfrog/build-info-go/utils"
"github.com/jfrog/gofrog/parallel"
"github.com/jfrog/jfrog-client-go/artifactory/services/fspatterns"
"github.com/jfrog/jfrog-client-go/artifactory/services/utils"
Expand Down Expand Up @@ -190,12 +190,12 @@ func createProperties(artifact clientutils.Artifact, uploadParams UploadParams)
}
// If Symlink target exists -> get SHA1 if isn't a directory
} else if !fileInfo.IsDir() {
var checksums map[biutils.Algorithm]string
checksums, err := biutils.GetFileChecksums(artifact.LocalPath, biutils.SHA1)
var checksums map[crypto.Algorithm]string
checksums, err := crypto.GetFileChecksums(artifact.LocalPath, crypto.SHA1)
if err != nil {
return nil, errorutils.CheckError(err)
}
artifactProps.AddProperty(utils.SymlinkSha1, checksums[biutils.SHA1])
artifactProps.AddProperty(utils.SymlinkSha1, checksums[crypto.SHA1])
}
artifactProps.AddProperty(utils.ArtifactorySymlink, artifactSymlink)
}
Expand Down
8 changes: 4 additions & 4 deletions artifactory/services/utils/multipartupload.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/jfrog/gofrog/crypto"
"io"
"net/http"
"os"
Expand All @@ -13,7 +14,6 @@ import (
"sync/atomic"
"time"

biUtils "github.com/jfrog/build-info-go/utils"
"github.com/jfrog/gofrog/parallel"
"github.com/jfrog/jfrog-client-go/auth"
"github.com/jfrog/jfrog-client-go/http/jfroghttpclient"
Expand Down Expand Up @@ -159,11 +159,11 @@ func (mu *MultipartUpload) UploadFileConcurrently(localPath, targetPath string,
}

if sha1 == "" {
var checksums map[biUtils.Algorithm]string
if checksums, err = biUtils.GetFileChecksums(localPath); errorutils.CheckError(err) != nil {
var checksums map[crypto.Algorithm]string
if checksums, err = crypto.GetFileChecksums(localPath); errorutils.CheckError(err) != nil {
return
}
sha1 = checksums[biUtils.SHA1]
sha1 = checksums[crypto.SHA1]
}

if progress != nil {
Expand Down
4 changes: 1 addition & 3 deletions artifactory/services/utils/searchutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,9 +395,7 @@ func (item ResultItem) GetItemRelativePath() string {
if item.Path == "." {
return path.Join(item.Repo, item.Name)
}

url := item.Repo
url = path.Join(url, item.Path, item.Name)
url := path.Join(item.Repo, item.Path, item.Name)
if item.Type == string(Folder) {
url = appendFolderSuffix(url)
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,6 @@ require (
gopkg.in/yaml.v3 v3.0.1 // indirect
)

// replace github.com/jfrog/build-info-go => github.com/jfrog/build-info-go v1.8.9-0.20240729133409-38a2c49a0d75
replace github.com/jfrog/build-info-go => github.com/jfrog/build-info-go v1.8.9-0.20240804091815-7407ceb49077

// replace github.com/jfrog/gofrog => github.com/jfrog/gofrog dev
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOl
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo=
github.com/jfrog/archiver/v3 v3.6.1 h1:LOxnkw9pOn45DzCbZNFV6K0+6dCsQ0L8mR3ZcujO5eI=
github.com/jfrog/archiver/v3 v3.6.1/go.mod h1:VgR+3WZS4N+i9FaDwLZbq+jeU4B4zctXL+gL4EMzfLw=
github.com/jfrog/build-info-go v1.9.31 h1:1pLC19hc9AEdWA87D+EcvMTLsDeMa390Z8TrNpCO4K8=
github.com/jfrog/build-info-go v1.9.31/go.mod h1:DZCElS/UhaSJHn0K1YzRUOJqiqVS4bjAEnGQSFncwNw=
github.com/jfrog/build-info-go v1.8.9-0.20240804091815-7407ceb49077 h1:Cdi9S8nUFMllxukuw5Z0X/UDmkW5nTdl0pT432fySp4=
github.com/jfrog/build-info-go v1.8.9-0.20240804091815-7407ceb49077/go.mod h1:DZCElS/UhaSJHn0K1YzRUOJqiqVS4bjAEnGQSFncwNw=
github.com/jfrog/gofrog v1.7.5 h1:dFgtEDefJdlq9cqTRoe09RLxS5Bxbe1Ev5+E6SmZHcg=
github.com/jfrog/gofrog v1.7.5/go.mod h1:jyGiCgiqSSR7k86hcUSu67XVvmvkkgWTmPsH25wI298=
github.com/kevinburke/ssh_config v1.2.0 h1:x584FjTGwHzMwvHx18PXxbBVzfnxogHaAReU4gf13a4=
Expand Down
7 changes: 4 additions & 3 deletions utils/io/fileutils/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"bytes"
"encoding/json"
"errors"
biutils "github.com/jfrog/build-info-go/utils"
"io"
"net/url"
"os"
Expand All @@ -14,7 +15,7 @@ import (
"strings"

"github.com/jfrog/build-info-go/entities"
biutils "github.com/jfrog/build-info-go/utils"
"github.com/jfrog/gofrog/crypto"
gofrog "github.com/jfrog/gofrog/io"
"github.com/jfrog/jfrog-client-go/utils/errorutils"
)
Expand Down Expand Up @@ -396,11 +397,11 @@ func GetFileDetailsFromReader(reader io.Reader, includeChecksums bool) (details
}

func calcChecksumDetailsFromReader(reader io.Reader) (entities.Checksum, error) {
checksums, err := biutils.CalcChecksums(reader)
checksums, err := crypto.CalcChecksums(reader)
if err != nil {
return entities.Checksum{}, errorutils.CheckError(err)
}
return entities.Checksum{Md5: checksums[biutils.MD5], Sha1: checksums[biutils.SHA1], Sha256: checksums[biutils.SHA256]}, nil
return entities.Checksum{Md5: checksums[crypto.MD5], Sha1: checksums[crypto.SHA1], Sha256: checksums[crypto.SHA256]}, nil
}

type FileDetails struct {
Expand Down
35 changes: 25 additions & 10 deletions utils/io/fileutils/temp.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package fileutils

import (
"fmt"
"os"
"path"
"strconv"
Expand Down Expand Up @@ -87,13 +88,13 @@ func CleanOldDirs() error {
for _, file := range files {
fileName := file.Name()
if strings.HasPrefix(fileName, tempPrefix) {
var timeStamp time.Time
timeStamp, err = extractTimestamp(fileName)
var timestamp time.Time
timestamp, err = extractTimestamp(fileName)
if err != nil {
return err
return errorutils.CheckErrorf("could not extract timestamp from file %s: %q", fileName, err)
}
// Delete old file/dirs.
if now.Sub(timeStamp).Hours() > maxFileAge {
if now.Sub(timestamp).Hours() > maxFileAge {
if err = RemovePath(path.Join(tempDirBase, fileName)); err != nil {
return err
}
Expand All @@ -104,15 +105,29 @@ func CleanOldDirs() error {
}

func extractTimestamp(item string) (time.Time, error) {
// Get timestamp from file/dir.
// Get the index of the last dash
endTimestampIdx := strings.LastIndex(item, "-")
if endTimestampIdx == -1 {
return time.Time{}, fmt.Errorf("invalid format: no dash found")
}

// Get the index of the dash before the last dash
beginningTimestampIdx := strings.LastIndex(item[:endTimestampIdx], "-")
if beginningTimestampIdx == -1 {
return time.Time{}, fmt.Errorf("invalid format: only one dash found")
}

// Extract the timestamp string
timestampStr := item[beginningTimestampIdx+1 : endTimestampIdx]
// Convert to int.
timeStampInt, err := strconv.ParseInt(timestampStr, 10, 64)
if len(timestampStr) == 0 {
return time.Time{}, fmt.Errorf("invalid format: empty timestamp")
}

// Convert to int
timestampInt, err := strconv.ParseInt(timestampStr, 10, 64)
if err != nil {
return time.Time{}, errorutils.CheckError(err)
return time.Time{}, fmt.Errorf("error parsing timestamp: %v", err)
}
// Convert to time type.
return time.Unix(timeStampInt, 0), nil
// Convert to time type
return time.Unix(timestampInt, 0), nil
}
39 changes: 29 additions & 10 deletions utils/io/fileutils/temp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package fileutils
import (
"os"
"testing"
"time"

"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -41,17 +42,35 @@ func TestCleanOldDirs(t *testing.T) {
}

func TestExtractTimestamp(t *testing.T) {
// Extract time from a file.
fileName := "jfrog.cli.temp.-008652489-1595147819.json"
timeStamp, err := extractTimestamp(fileName)
assert.NoError(t, err)
assert.Equal(t, int64(8652489), timeStamp.Unix())
testCases := []struct {
item string
expectedTime time.Time
expectError bool
}{
// Valid cases
{"jfrog.cli.temp.prefix-1625097600-suffix", time.Unix(1625097600, 0), false},
{"jfrog.cli.temp.some-1234567890-other", time.Unix(1234567890, 0), false},

// Extract time from a dir.
fileName = "asd-asjfrog.cli.temp.-008652489-1595147444"
timeStamp, err = extractTimestamp(fileName)
assert.NoError(t, err)
assert.Equal(t, int64(8652489), timeStamp.Unix())
// Invalid cases
{"jfrog.cli.temp.no-dash", time.Time{}, true},
{"jfrog.cli.temp.one-dash-1234567890", time.Time{}, true},
{"jfrog.cli.temp.two-dashes--", time.Time{}, true},
{"jfrog.cli.temp.prefix--suffix", time.Time{}, true},
{"jfrog.cli.temp.prefix-abc-suffix", time.Time{}, true},
{"jfrog.cli.temp.prefix-1625097600suffix", time.Time{}, true},
}

for _, test := range testCases {
t.Run(test.item, func(t *testing.T) {
result, err := extractTimestamp(test.item)
if (err != nil) != test.expectError {
t.Errorf("expected error: %v, got: %v", test.expectError, err)
}
if !result.Equal(test.expectedTime) {
t.Errorf("expected time: %v, got: %v", test.expectedTime, result)
}
})
}
}

func AssertFileExists(t *testing.T, name string) {
Expand Down
Loading