From fa1ead35e21d3eb8158b776d1fc93cb3444baba4 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Tue, 26 Apr 2016 09:56:11 -0700 Subject: [PATCH] More occurrences of -1 and fixup comments Signed-off-by: Riyaz Faizullabhoy --- client/client.go | 6 +++--- tuf/store/filestore.go | 2 +- tuf/store/filestore_test.go | 4 ++-- tuf/store/httpstore.go | 3 ++- tuf/store/httpstore_test.go | 2 +- tuf/store/memorystore.go | 7 ++++--- tuf/store/memorystore_test.go | 2 +- tuf/testutils/swizzler.go | 20 ++++++++++---------- 8 files changed, 24 insertions(+), 22 deletions(-) diff --git a/client/client.go b/client/client.go index 4455b62776..391b754526 100644 --- a/client/client.go +++ b/client/client.go @@ -649,7 +649,7 @@ func (r *NotaryRepository) bootstrapRepo() error { logrus.Debugf("Loading trusted collection.") for _, role := range data.BaseRoles { - jsonBytes, err := r.fileStore.GetMeta(role, -1) + jsonBytes, err := r.fileStore.GetMeta(role, store.MaxSize) if err != nil { if _, ok := err.(store.ErrMetaNotFound); ok && // server snapshots are supported, and server timestamp management @@ -781,7 +781,7 @@ func (r *NotaryRepository) bootstrapClient(checkInitialized bool) (*tufclient.Cl // during update which will cause us to download a new root and perform a rotation. // If we have an old root, and it's valid, then we overwrite the newBuilder to be one // preloaded with the old root or one which uses the old root for trust bootstrapping. - if rootJSON, err := r.fileStore.GetMeta(data.CanonicalRootRole, -1); err == nil { + if rootJSON, err := r.fileStore.GetMeta(data.CanonicalRootRole, store.MaxSize); err == nil { // if we can't load the cached root, fail hard because that is how we pin trust if err := oldBuilder.Load(data.CanonicalRootRole, rootJSON, minVersion, true); err != nil { return nil, err @@ -808,7 +808,7 @@ func (r *NotaryRepository) bootstrapClient(checkInitialized bool) (*tufclient.Cl // if remote store successfully set up, try and get root from remote // We don't have any local data to determine the size of root, so try the maximum (though it is restricted at 100MB) - tmpJSON, err := remote.GetMeta(data.CanonicalRootRole, -1) + tmpJSON, err := remote.GetMeta(data.CanonicalRootRole, store.MaxSize) if err != nil { // we didn't have a root in cache and were unable to load one from // the server. Nothing we can do but error. diff --git a/tuf/store/filestore.go b/tuf/store/filestore.go index d5cd0baa6a..21e9fbd697 100644 --- a/tuf/store/filestore.go +++ b/tuf/store/filestore.go @@ -39,7 +39,7 @@ func (f *FilesystemStore) getPath(name string) string { } // GetMeta returns the meta for the given name (a role) up to size bytes -// If size is -1, this corresponds to "infinite," but we cut off at the +// If size is "MaxSize", this corresponds to "infinite," but we cut off at a // predefined threshold "notary.MaxDownloadSize". func (f *FilesystemStore) GetMeta(name string, size int64) ([]byte, error) { meta, err := ioutil.ReadFile(f.getPath(name)) diff --git a/tuf/store/filestore_test.go b/tuf/store/filestore_test.go index a3db850074..72c705bba8 100644 --- a/tuf/store/filestore_test.go +++ b/tuf/store/filestore_test.go @@ -85,8 +85,8 @@ func TestGetMeta(t *testing.T) { require.Equal(t, testContent, content, "Content read from file was corrupted.") - // Check that -1 size reads everything - content, err = s.GetMeta("testMeta", int64(-1)) + // Check that MaxSize size reads everything + content, err = s.GetMeta("testMeta", MaxSize) require.Nil(t, err, "GetMeta returned unexpected error: %v", err) require.Equal(t, testContent, content, "Content read from file was corrupted.") diff --git a/tuf/store/httpstore.go b/tuf/store/httpstore.go index 25bce062c8..988ede1621 100644 --- a/tuf/store/httpstore.go +++ b/tuf/store/httpstore.go @@ -139,7 +139,8 @@ func translateStatusToError(resp *http.Response, resource string) error { // GetMeta downloads the named meta file with the given size. A short body // is acceptable because in the case of timestamp.json, the size is a cap, // not an exact length. -// If size is -1, this corresponds to "infinite," but we cut off at 100MB +// If size is "MaxSize", this corresponds to "infinite," but we cut off at a +// predefined threshold "notary.MaxDownloadSize". func (s HTTPStore) GetMeta(name string, size int64) ([]byte, error) { url, err := s.buildMetaURL(name) if err != nil { diff --git a/tuf/store/httpstore_test.go b/tuf/store/httpstore_test.go index 90daaa8e6e..372a46deef 100644 --- a/tuf/store/httpstore_test.go +++ b/tuf/store/httpstore_test.go @@ -95,7 +95,7 @@ func TestHTTPStoreGetAllMeta(t *testing.T) { if err != nil { t.Fatal(err) } - j, err := store.GetMeta("root", -1) + j, err := store.GetMeta("root", MaxSize) if err != nil { t.Fatal(err) } diff --git a/tuf/store/memorystore.go b/tuf/store/memorystore.go index a6954da6fb..cb76a88522 100644 --- a/tuf/store/memorystore.go +++ b/tuf/store/memorystore.go @@ -39,9 +39,10 @@ type MemoryStore struct { } // GetMeta returns up to size bytes of data references by name. -// If size is -1, this corresponds to "infinite," but we cut off at 100MB -// as we will always know the size for everything but a timestamp and -// sometimes a root, neither of which should be exceptionally large +// If size is "MaxSize", this corresponds to "infinite," but we cut off at a +// predefined threshold "notary.MaxDownloadSize", as we will always know the +// size for everything but a timestamp and sometimes a root, +// neither of which should be exceptionally large func (m *MemoryStore) GetMeta(name string, size int64) ([]byte, error) { d, ok := m.meta[name] if ok { diff --git a/tuf/store/memorystore_test.go b/tuf/store/memorystore_test.go index 8ebe0a005b..0ff06b5d5e 100644 --- a/tuf/store/memorystore_test.go +++ b/tuf/store/memorystore_test.go @@ -63,7 +63,7 @@ func TestMemoryStoreGetMetaSize(t *testing.T) { require.NoError(t, err) require.Equal(t, []byte{}, meta) - // we can get the whole thing by passing -1 + // we can get the whole thing by passing MaxSize (-1) meta, err = s.GetMeta("content", MaxSize) require.NoError(t, err) require.Equal(t, content, meta) diff --git a/tuf/testutils/swizzler.go b/tuf/testutils/swizzler.go index 2f746c81d5..dd004b7059 100644 --- a/tuf/testutils/swizzler.go +++ b/tuf/testutils/swizzler.go @@ -89,7 +89,7 @@ func serializeMetadata(cs signed.CryptoService, s *data.Signed, role string, // gets a Signed from the metadata store func signedFromStore(cache store.MetadataStore, role string) (*data.Signed, error) { - b, err := cache.GetMeta(role, -1) + b, err := cache.GetMeta(role, store.MaxSize) if err != nil { return nil, err } @@ -122,7 +122,7 @@ func NewMetadataSwizzler(gun string, initialMetadata map[string][]byte, // SetInvalidJSON corrupts metadata into something that is no longer valid JSON func (m *MetadataSwizzler) SetInvalidJSON(role string) error { - metaBytes, err := m.MetadataCache.GetMeta(role, -1) + metaBytes, err := m.MetadataCache.GetMeta(role, store.MaxSize) if err != nil { return err } @@ -133,7 +133,7 @@ func (m *MetadataSwizzler) SetInvalidJSON(role string) error { // JSON bytes, which should not affect serialization, but will change the checksum // of the file. func (m *MetadataSwizzler) AddExtraSpace(role string) error { - metaBytes, err := m.MetadataCache.GetMeta(role, -1) + metaBytes, err := m.MetadataCache.GetMeta(role, store.MaxSize) if err != nil { return err } @@ -357,7 +357,7 @@ func (m *MetadataSwizzler) SetThreshold(role string, newThreshold int) error { roleSpecifier = path.Dir(role) } - b, err := m.MetadataCache.GetMeta(roleSpecifier, -1) + b, err := m.MetadataCache.GetMeta(roleSpecifier, store.MaxSize) if err != nil { return err } @@ -413,7 +413,7 @@ func (m *MetadataSwizzler) RotateKey(role string, key data.PublicKey) error { roleSpecifier = path.Dir(role) } - b, err := m.MetadataCache.GetMeta(roleSpecifier, -1) + b, err := m.MetadataCache.GetMeta(roleSpecifier, store.MaxSize) if err != nil { return err } @@ -471,7 +471,7 @@ func (m *MetadataSwizzler) ChangeRootKey() error { return err } - b, err := m.MetadataCache.GetMeta(data.CanonicalRootRole, -1) + b, err := m.MetadataCache.GetMeta(data.CanonicalRootRole, store.MaxSize) if err != nil { return err } @@ -509,7 +509,7 @@ func (m *MetadataSwizzler) UpdateSnapshotHashes(roles ...string) error { snapshotSigned *data.Signed err error ) - if metaBytes, err = m.MetadataCache.GetMeta(data.CanonicalSnapshotRole, -1); err != nil { + if metaBytes, err = m.MetadataCache.GetMeta(data.CanonicalSnapshotRole, store.MaxSize); err != nil { return err } @@ -525,7 +525,7 @@ func (m *MetadataSwizzler) UpdateSnapshotHashes(roles ...string) error { for _, role := range roles { if role != data.CanonicalSnapshotRole && role != data.CanonicalTimestampRole { - if metaBytes, err = m.MetadataCache.GetMeta(role, -1); err != nil { + if metaBytes, err = m.MetadataCache.GetMeta(role, store.MaxSize); err != nil { return err } @@ -561,7 +561,7 @@ func (m *MetadataSwizzler) UpdateTimestampHash() error { timestampSigned *data.Signed err error ) - if metaBytes, err = m.MetadataCache.GetMeta(data.CanonicalTimestampRole, -1); err != nil { + if metaBytes, err = m.MetadataCache.GetMeta(data.CanonicalTimestampRole, store.MaxSize); err != nil { return err } // we can't just create a new timestamp, because then the expiry would be @@ -570,7 +570,7 @@ func (m *MetadataSwizzler) UpdateTimestampHash() error { return err } - if metaBytes, err = m.MetadataCache.GetMeta(data.CanonicalSnapshotRole, -1); err != nil { + if metaBytes, err = m.MetadataCache.GetMeta(data.CanonicalSnapshotRole, store.MaxSize); err != nil { return err }