-
Notifications
You must be signed in to change notification settings - Fork 96
Remove archiver xz dependencies #50
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
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
704c293
Remove extra debug log
d7f16ad
Merge remote-tracking branch 'origin/master'
fergusstrange ca2c428
Merge remote-tracking branch 'origin/master'
fergusstrange ca2cf12
Merge remote-tracking branch 'origin/master'
fergusstrange 70dbd33
Merge remote-tracking branch 'origin/master'
fergusstrange 7ac6bc1
Try removing dependencies on chunky decompression libs
fergusstrange 2cd93da
Try removing dependencies on chunky decompression libs
fergusstrange 1155993
Merge remote-tracking branch 'origin/remove_archiver_xz_dependencies'…
fergusstrange 62f5b22
Move around error messaging
fergusstrange c3f9f76
Resolve some of the broken tests
fergusstrange fc52fea
Resolve last batch of failing tests
fergusstrange 3721d93
Improve coverage and clean up decompression code
fergusstrange 065bf49
Move to using a non default port for validating create default test d…
fergusstrange cb9166d
Try rolling back all changes to see if issues are around new decompre…
fergusstrange d0805f9
Put back code without timeout on tests now
fergusstrange 3dd098b
Fix param to go 1.13 standard
fergusstrange f0dc09d
Add a little more coverage and strategy to improve
fergusstrange ec2fb25
Add further decompression tests
fergusstrange File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| package embeddedpostgres | ||
|
|
||
| import ( | ||
| "archive/tar" | ||
| "fmt" | ||
| "io" | ||
| "os" | ||
| "path/filepath" | ||
|
|
||
| "github.com/xi2/xz" | ||
| ) | ||
|
|
||
| func defaultTarReader(xzReader *xz.Reader) (func() (*tar.Header, error), func() io.Reader) { | ||
| tarReader := tar.NewReader(xzReader) | ||
|
|
||
| return func() (*tar.Header, error) { | ||
| return tarReader.Next() | ||
| }, func() io.Reader { | ||
| return tarReader | ||
| } | ||
| } | ||
|
|
||
| func decompressTarXz(tarReader func(*xz.Reader) (func() (*tar.Header, error), func() io.Reader), path, extractPath string) error { | ||
| tarFile, err := os.Open(path) | ||
| if err != nil { | ||
| return errorUnableToExtract(path, extractPath) | ||
| } | ||
|
|
||
| defer func() { | ||
| if err := tarFile.Close(); err != nil { | ||
| panic(err) | ||
| } | ||
| }() | ||
|
|
||
| xzReader, err := xz.NewReader(tarFile, 0) | ||
| if err != nil { | ||
| return errorUnableToExtract(path, extractPath) | ||
| } | ||
|
|
||
| readNext, reader := tarReader(xzReader) | ||
|
|
||
| for { | ||
| header, err := readNext() | ||
|
|
||
| if err == io.EOF { | ||
| return nil | ||
| } | ||
|
|
||
| if err != nil { | ||
| return errorExtractingPostgres(err) | ||
| } | ||
|
|
||
| targetPath := filepath.Join(extractPath, header.Name) | ||
|
|
||
| if err := os.MkdirAll(filepath.Dir(targetPath), 0755); err != nil { | ||
| return errorExtractingPostgres(err) | ||
| } | ||
|
|
||
| switch header.Typeflag { | ||
| case tar.TypeReg: | ||
| outFile, err := os.OpenFile(targetPath, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode)) | ||
| if err != nil { | ||
| return errorExtractingPostgres(err) | ||
| } | ||
|
|
||
| if _, err := io.Copy(outFile, reader()); err != nil { | ||
| return errorExtractingPostgres(err) | ||
| } | ||
|
|
||
| if err := outFile.Close(); err != nil { | ||
| return errorExtractingPostgres(err) | ||
| } | ||
| case tar.TypeSymlink: | ||
| if err := os.RemoveAll(targetPath); err != nil { | ||
| return errorExtractingPostgres(err) | ||
| } | ||
|
|
||
| if err := os.Symlink(header.Linkname, targetPath); err != nil { | ||
| return errorExtractingPostgres(err) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func errorUnableToExtract(cacheLocation, binariesPath string) error { | ||
| return fmt.Errorf("unable to extract postgres archive %s to %s, if running parallel tests, configure RuntimePath to isolate testing directories", cacheLocation, binariesPath) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| package embeddedpostgres | ||
|
|
||
| import ( | ||
| "archive/tar" | ||
| "errors" | ||
| "io" | ||
| "io/ioutil" | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/xi2/xz" | ||
| ) | ||
|
|
||
| func Test_decompressTarXz(t *testing.T) { | ||
| tempDir, err := ioutil.TempDir("", "temp_tar_test") | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| archive, cleanUp := createTempXzArchive() | ||
| defer cleanUp() | ||
|
|
||
| err = decompressTarXz(defaultTarReader, archive, tempDir) | ||
|
|
||
| assert.NoError(t, err) | ||
|
|
||
| expectedExtractedFileLocation := filepath.Join(tempDir, "dir1", "dir2", "some_content") | ||
| assert.FileExists(t, expectedExtractedFileLocation) | ||
|
|
||
| fileContentBytes, err := ioutil.ReadFile(expectedExtractedFileLocation) | ||
| assert.NoError(t, err) | ||
|
|
||
| assert.Equal(t, "b33r is g00d", string(fileContentBytes)) | ||
| } | ||
|
|
||
| func Test_decompressTarXz_ErrorWhenFileNotExists(t *testing.T) { | ||
| err := decompressTarXz(defaultTarReader, "/does-not-exist", "/also-fake") | ||
|
|
||
| assert.EqualError(t, err, "unable to extract postgres archive /does-not-exist to /also-fake, if running parallel tests, configure RuntimePath to isolate testing directories") | ||
| } | ||
|
|
||
| func Test_decompressTarXz_ErrorWhenErrorDuringRead(t *testing.T) { | ||
| tempDir, err := ioutil.TempDir("", "temp_tar_test") | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| archive, cleanUp := createTempXzArchive() | ||
| defer cleanUp() | ||
|
|
||
| err = decompressTarXz(func(reader *xz.Reader) (func() (*tar.Header, error), func() io.Reader) { | ||
| return func() (*tar.Header, error) { | ||
| return nil, errors.New("oh noes") | ||
| }, nil | ||
| }, archive, tempDir) | ||
|
|
||
| assert.EqualError(t, err, "unable to extract postgres archive: oh noes") | ||
| } | ||
|
|
||
| func Test_decompressTarXz_ErrorWhenFailedToReadFileToCopy(t *testing.T) { | ||
| tempDir, err := ioutil.TempDir("", "temp_tar_test") | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| archive, cleanUp := createTempXzArchive() | ||
| defer cleanUp() | ||
|
|
||
| blockingFile := filepath.Join(tempDir, "blocking") | ||
|
|
||
| if err = ioutil.WriteFile(blockingFile, []byte("wazz"), 0000); err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| fileBlockingExtractTarReader := func(reader *xz.Reader) (func() (*tar.Header, error), func() io.Reader) { | ||
| shouldReadFile := true | ||
|
|
||
| return func() (*tar.Header, error) { | ||
| if shouldReadFile { | ||
| shouldReadFile = false | ||
|
|
||
| return &tar.Header{ | ||
| Typeflag: tar.TypeReg, | ||
| Name: "blocking", | ||
| }, nil | ||
| } | ||
|
|
||
| return nil, io.EOF | ||
| }, func() io.Reader { | ||
| open, _ := os.Open("file_not_exists") | ||
| return open | ||
| } | ||
| } | ||
|
|
||
| err = decompressTarXz(fileBlockingExtractTarReader, archive, tempDir) | ||
|
|
||
| assert.Regexp(t, "^unable to extract postgres archive:.+$", err) | ||
| } | ||
|
|
||
| func Test_decompressTarXz_ErrorWhenFileToCopyToNotExists(t *testing.T) { | ||
| tempDir, err := ioutil.TempDir("", "temp_tar_test") | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| archive, cleanUp := createTempXzArchive() | ||
| defer cleanUp() | ||
|
|
||
| fileBlockingExtractTarReader := func(reader *xz.Reader) (func() (*tar.Header, error), func() io.Reader) { | ||
| shouldReadFile := true | ||
|
|
||
| return func() (*tar.Header, error) { | ||
| if shouldReadFile { | ||
| shouldReadFile = false | ||
|
|
||
| return &tar.Header{ | ||
| Typeflag: tar.TypeReg, | ||
| Name: "some_dir/wazz/dazz/fazz", | ||
| }, nil | ||
| } | ||
|
|
||
| return nil, io.EOF | ||
| }, func() io.Reader { | ||
| open, _ := os.Open("file_not_exists") | ||
| return open | ||
| } | ||
| } | ||
|
|
||
| err = decompressTarXz(fileBlockingExtractTarReader, archive, tempDir) | ||
|
|
||
| assert.Regexp(t, "^unable to extract postgres archive:.+$", err) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit worried that you're going to use an xz library from 2017 (!!).
It seems like https://github.com/ulikunitz/xz is more recommended... (it's even recommended on the README.md of the above library)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this is the library we've been using under the hood from mholt archiver, and is a lighter weight version of ulikunitz/xz when we only need decompression. The README suggests to use ulikunitz/xz when compression is needed, which we can get away without.
This should help us avoid any issues on migration as like decompression algorithm won't have changed, and avoid having to track any of the CVEs which keep cropping up in mholt/archiver. Happy to assess this moving forward if this one crops up in any vulnerability scans or there are issues but it appears to have been a pretty rock solid library for that long!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that we don't use the xz capabilities (as since my last PR, we're setting
binariesPathmanually), so - whatever I say here is not going to affect our usage at all.With that said, I feel like this library is completely abandoned. It's old (as noted), and doesn't even use go.mod (it doesn't have any dependencies, but still).
I understand that a "heavier" package, which also supports compression might be more exposed to potential vulnerabilities, but with ulikunitz/xz, I at least feel like - if there will be one - it'll get fixed (e.g. ulikunitz/xz@69c6093).
As noted above - your decision here will not affect me in any way - so - make sure it makes sense to what you're going to use it for...
Thanks!
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a library hasn't been updated in a while it can also be a good thing - it can mean the API is stable/feature complete and there are no bugs.