Skip to content

Commit

Permalink
Fix gob encoding of system/package struct data (elastic#18887)
Browse files Browse the repository at this point in the history
The system/package dataset uses encoding/gob to encode and decode
the Package struct data that it persists. The struct was updated to contain
an error which is an interface type that's not serializable by default.

I have made the field unexpected so that it is ignored by encoding/gob.

Fixes elastic#18536
  • Loading branch information
andrewkroh authored and melchiormoulin committed Oct 14, 2020
1 parent e7e8e88 commit ad55833
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- system/socket: Fixed compatibility issue with kernel 5.x. {pull}15771[15771]
- system/package: Fix parsing of Installed-Size field of DEB packages. {issue}16661[16661] {pull}17188[17188]
- system module: Fix panic during initialisation when /proc/stat can't be read. {pull}17569[17569]
- system/package: Fix an error that can occur while trying to persist package metadata. {issue}18536[18536] {pull}18887[18887]

*Filebeat*

Expand Down
7 changes: 4 additions & 3 deletions x-pack/auditbeat/module/system/package/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,9 @@ type Package struct {
Size uint64
Summary string
URL string
Error error
Type string

error error
}

// Hash creates a hash for Package.
Expand Down Expand Up @@ -389,8 +390,8 @@ func (ms *MetricSet) packageEvent(pkg *Package, eventType string, action eventAc
event.MetricSetFields.Put("entity_id", pkg.entityID(ms.HostID()))
}

if pkg.Error != nil {
event.RootFields.Put("error.message", pkg.Error.Error())
if pkg.error != nil {
event.RootFields.Put("error.message", pkg.error.Error())
}

return event
Expand Down
8 changes: 4 additions & 4 deletions x-pack/auditbeat/module/system/package/package_homebrew.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ func listBrewPackages() ([]*Package, error) {
installReceiptPath := path.Join(homebrewCellarPath, pkg.Name, pkg.Version, "INSTALL_RECEIPT.json")
contents, err := ioutil.ReadFile(installReceiptPath)
if err != nil {
pkg.Error = errors.Wrapf(err, "error reading %v", installReceiptPath)
pkg.error = errors.Wrapf(err, "error reading %v", installReceiptPath)
} else {
var installReceipt InstallReceipt
err = json.Unmarshal(contents, &installReceipt)
if err != nil {
pkg.Error = errors.Wrapf(err, "error unmarshalling JSON in %v", installReceiptPath)
pkg.error = errors.Wrapf(err, "error unmarshalling JSON in %v", installReceiptPath)
} else {
formulaPath = installReceipt.Source.Path
}
Expand All @@ -78,7 +78,7 @@ func listBrewPackages() ([]*Package, error) {

file, err := os.Open(formulaPath)
if err != nil {
pkg.Error = errors.Wrapf(err, "error reading %v", formulaPath)
pkg.error = errors.Wrapf(err, "error reading %v", formulaPath)
} else {
defer file.Close()

Expand All @@ -97,7 +97,7 @@ func listBrewPackages() ([]*Package, error) {
}
}
if err = scanner.Err(); err != nil {
pkg.Error = errors.Wrapf(err, "error parsing %v", formulaPath)
pkg.error = errors.Wrapf(err, "error parsing %v", formulaPath)
}
}

Expand Down
73 changes: 73 additions & 0 deletions x-pack/auditbeat/module/system/package/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,15 @@
package pkg

import (
"bytes"
"encoding/gob"
"errors"
"flag"
"io/ioutil"
"path/filepath"
"runtime"
"testing"
"time"

"github.com/stretchr/testify/assert"

Expand All @@ -19,6 +25,8 @@ import (
mbtest "github.com/elastic/beats/v7/metricbeat/mb/testing"
)

var flagUpdateGob = flag.Bool("update-gob", false, "update persisted gob testdata")

func TestData(t *testing.T) {
if runtime.GOOS == "darwin" {
t.Skip("FIXME: https://github.com/elastic/beats/issues/18855")
Expand Down Expand Up @@ -160,3 +168,68 @@ func getConfig() map[string]interface{} {
"datasets": []string{"package"},
}
}

func TestPackageGobEncodeDecode(t *testing.T) {
pkg := Package{
Name: "foo",
Version: "1.2.3",
Release: "1",
Arch: "amd64",
License: "bar",
InstallTime: time.Unix(1591021924, 0).UTC(),
Size: 1234,
Summary: "Foo stuff",
URL: "http://foo.example.com",
Type: "rpm",
}

buf := new(bytes.Buffer)
if err := gob.NewEncoder(buf).Encode(pkg); err != nil {
t.Fatal(err)
}

const gobTestFile = "testdata/package.v1.gob"
if *flagUpdateGob {
// NOTE: If you are updating this file then you may have introduced a
// a breaking change.
if err := ioutil.WriteFile(gobTestFile, buf.Bytes(), 0644); err != nil {
t.Fatal(err)
}
}

t.Run("decode", func(t *testing.T) {
var pkgDecoded Package
if err := gob.NewDecoder(buf).Decode(&pkgDecoded); err != nil {
t.Fatal(err)
}
assert.Equal(t, pkg, pkgDecoded)
})

// Validate that we get the same result when decoding an earlier saved version.
// This detects breakages to the struct or to the encoding/decoding pkgs.
t.Run("decode_from_file", func(t *testing.T) {
contents, err := ioutil.ReadFile(gobTestFile)
if err != nil {
t.Fatal(err)
}

var pkgDecoded Package
if err := gob.NewDecoder(bytes.NewReader(contents)).Decode(&pkgDecoded); err != nil {
t.Fatal(err)
}
assert.Equal(t, pkg, pkgDecoded)
})
}

// Regression test for https://github.com/elastic/beats/issues/18536 to verify
// that error isn't made public.
func TestPackageWithErrorGobEncode(t *testing.T) {
pkg := Package{
error: errors.New("test"),
}

buf := new(bytes.Buffer)
if err := gob.NewEncoder(buf).Encode(pkg); err != nil {
t.Fatal(err)
}
}
2 changes: 1 addition & 1 deletion x-pack/auditbeat/module/system/package/rpm_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ func packageFromHeader(header C.Header, openedLibrpm *librpm) (*Package, error)
if version != nil {
pkg.Version = C.GoString(version)
} else {
pkg.Error = errors.New("Failed to get package version")
pkg.error = errors.New("failed to get package version")
}

pkg.Release = C.GoString(C.my_headerGetString(openedLibrpm.headerGetString, header, RPMTAG_RELEASE))
Expand Down
Binary file not shown.

0 comments on commit ad55833

Please sign in to comment.