From 802673fc9dbbce3c938773c43ffd51130ea838e3 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Fri, 26 Feb 2016 10:51:55 -0800 Subject: [PATCH] Add cache control headers to Getting metadata Signed-off-by: Ying Li --- server/handlers/default.go | 38 +++++++++++++++++- server/handlers/default_test.go | 63 +++++++++++++++++++++++++----- server/handlers/roles.go | 44 ++++++++++----------- server/handlers/roles_test.go | 8 +--- server/snapshot/snapshot.go | 22 ++++++----- server/snapshot/snapshot_test.go | 21 ++++++---- server/timestamp/timestamp.go | 27 +++++++------ server/timestamp/timestamp_test.go | 8 ++-- 8 files changed, 162 insertions(+), 69 deletions(-) diff --git a/server/handlers/default.go b/server/handlers/default.go index 96a601412c..1d43af467b 100644 --- a/server/handlers/default.go +++ b/server/handlers/default.go @@ -2,11 +2,16 @@ package handlers import ( "bytes" + "crypto/sha256" + "encoding/hex" "encoding/json" + "fmt" "io" "net/http" "strings" + "time" + "github.com/Sirupsen/logrus" ctxu "github.com/docker/distribution/context" "github.com/gorilla/mux" "golang.org/x/net/context" @@ -20,6 +25,14 @@ import ( "github.com/docker/notary/tuf/validation" ) +const ( + // ConsistentCacheMaxAge is the Cache-Control header's max age for consistent downloads + ConsistentCacheMaxAge int = 30 * 24 * 60 * 60 // 30 days + + // NonConsistentCacheMaxAge is the Cache-Control header's max age for current (non-consistent) downloads + NonConsistentCacheMaxAge int = 5 * 60 // five minutes +) + // MainHandler is the default handler for the server func MainHandler(ctx context.Context, w http.ResponseWriter, r *http.Request) error { // For now it only supports `GET` @@ -122,7 +135,30 @@ func getHandler(ctx context.Context, w http.ResponseWriter, r *http.Request, var return errors.ErrNoStorage.WithDetail(nil) } - return getRole(ctx, w, store, gun, tufRole, checksum) + creation, output, err := getRole(ctx, store, gun, tufRole, checksum) + if err != nil { + return err + } + if creation == nil { + // This shouldn't ever happen, but if it does, it just messes up the cache headers, so + // proceed anyway + logrus.Warnf("Got bytes out for %s's %s (checksum: %s), but missing creation date", + gun, tufRole, checksum) + creation = &time.Time{} // set the last modification date to the beginning of time + } + + maxAge := ConsistentCacheMaxAge + if checksum == "" { // maxAge should be much shorter for metadata without checksums + maxAge = NonConsistentCacheMaxAge + shasum := sha256.Sum256(output) + checksum = hex.EncodeToString(shasum[:]) + } + + w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%v", maxAge)) + w.Header().Set("Last-Modified", creation.Format(time.RFC1123)) + w.Header().Set("ETag", checksum) + w.Write(output) + return nil } // DeleteHandler deletes all data for a GUN. A 200 responses indicates success. diff --git a/server/handlers/default_test.go b/server/handlers/default_test.go index 1f729028ba..5b794e9a1f 100644 --- a/server/handlers/default_test.go +++ b/server/handlers/default_test.go @@ -2,6 +2,8 @@ package handlers import ( "bytes" + "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" "io/ioutil" @@ -172,6 +174,28 @@ func TestGetKeyHandlerCreatesOnce(t *testing.T) { } } +// Verifies that the body is as expected, the ETag is as expected, +func verifyGetResponse(t *testing.T, rw *httptest.ResponseRecorder, expectedBytes []byte, + getWithChecksum bool, checksumHex string) { + + body, err := ioutil.ReadAll(rw.Body) + assert.NoError(t, err) + assert.True(t, bytes.Equal(expectedBytes, body)) + + lastModified, err := time.Parse(time.RFC1123, rw.HeaderMap.Get("Last-Modified")) + assert.NoError(t, err) + assert.True(t, lastModified.After(time.Now().Add(-5*time.Minute))) + + cacheControl := rw.HeaderMap.Get("Cache-Control") + maxAge := NonConsistentCacheMaxAge + if getWithChecksum { + maxAge = ConsistentCacheMaxAge + } + assert.Equal(t, fmt.Sprintf("public, max-age=%v", maxAge), cacheControl) + + assert.Equal(t, rw.HeaderMap.Get("ETag"), checksumHex) +} + func TestGetHandlerRoot(t *testing.T) { metaStore := storage.NewMemStorage() repo, _, err := testutils.EmptyRepo("gun") @@ -194,10 +218,17 @@ func TestGetHandlerRoot(t *testing.T) { "tufRole": "root", } - rw := httptest.NewRecorder() + checksumBytes := sha256.Sum256(rootJSON) + checksumHex := hex.EncodeToString(checksumBytes[:]) - err = getHandler(ctx, rw, req, vars) - assert.NoError(t, err) + rw := httptest.NewRecorder() + assert.NoError(t, getHandler(ctx, rw, req, vars)) + verifyGetResponse(t, rw, rootJSON, false, checksumHex) + + vars["checksum"] = checksumHex + rw = httptest.NewRecorder() + assert.NoError(t, getHandler(ctx, rw, req, vars)) + verifyGetResponse(t, rw, rootJSON, true, checksumHex) } func TestGetHandlerTimestamp(t *testing.T) { @@ -228,10 +259,17 @@ func TestGetHandlerTimestamp(t *testing.T) { "tufRole": "timestamp", } - rw := httptest.NewRecorder() + checksumBytes := sha256.Sum256(tsJSON) + checksumHex := hex.EncodeToString(checksumBytes[:]) - err = getHandler(ctx, rw, req, vars) - assert.NoError(t, err) + rw := httptest.NewRecorder() + assert.NoError(t, getHandler(ctx, rw, req, vars)) + verifyGetResponse(t, rw, tsJSON, false, checksumHex) + + vars["checksum"] = checksumHex + rw = httptest.NewRecorder() + assert.NoError(t, getHandler(ctx, rw, req, vars)) + verifyGetResponse(t, rw, tsJSON, true, checksumHex) } func TestGetHandlerSnapshot(t *testing.T) { @@ -256,10 +294,17 @@ func TestGetHandlerSnapshot(t *testing.T) { "tufRole": "snapshot", } - rw := httptest.NewRecorder() + checksumBytes := sha256.Sum256(snJSON) + checksumHex := hex.EncodeToString(checksumBytes[:]) - err = getHandler(ctx, rw, req, vars) - assert.NoError(t, err) + rw := httptest.NewRecorder() + assert.NoError(t, getHandler(ctx, rw, req, vars)) + verifyGetResponse(t, rw, snJSON, false, checksumHex) + + vars["checksum"] = checksumHex + rw = httptest.NewRecorder() + assert.NoError(t, getHandler(ctx, rw, req, vars)) + verifyGetResponse(t, rw, snJSON, true, checksumHex) } func TestGetHandler404(t *testing.T) { diff --git a/server/handlers/roles.go b/server/handlers/roles.go index dac0efdbfc..415cf2bb85 100644 --- a/server/handlers/roles.go +++ b/server/handlers/roles.go @@ -1,7 +1,7 @@ package handlers import ( - "io" + "time" "golang.org/x/net/context" @@ -13,35 +13,35 @@ import ( "github.com/docker/notary/tuf/signed" ) -func getRole(ctx context.Context, w io.Writer, store storage.MetaStore, gun, role, checksum string) error { +func getRole(ctx context.Context, store storage.MetaStore, gun, role, checksum string) (*time.Time, []byte, error) { var ( - out []byte - err error + creation *time.Time + out []byte + err error ) if checksum == "" { // the timestamp and snapshot might be server signed so are // handled specially switch role { case data.CanonicalTimestampRole, data.CanonicalSnapshotRole: - return getMaybeServerSigned(ctx, w, store, gun, role) + return getMaybeServerSigned(ctx, store, gun, role) } - _, out, err = store.GetCurrent(gun, role) + creation, out, err = store.GetCurrent(gun, role) } else { - _, out, err = store.GetChecksum(gun, role, checksum) + creation, out, err = store.GetChecksum(gun, role, checksum) } if err != nil { if _, ok := err.(storage.ErrNotFound); ok { - return errors.ErrMetadataNotFound.WithDetail(err) + return nil, nil, errors.ErrMetadataNotFound.WithDetail(err) } - return errors.ErrUnknown.WithDetail(err) + return nil, nil, errors.ErrUnknown.WithDetail(err) } if out == nil { - return errors.ErrMetadataNotFound.WithDetail(nil) + return nil, nil, errors.ErrMetadataNotFound.WithDetail(nil) } - w.Write(out) - return nil + return creation, out, nil } // getMaybeServerSigned writes the current snapshot or timestamp (based on the @@ -49,32 +49,32 @@ func getRole(ctx context.Context, w io.Writer, store storage.MetaStore, gun, rol // the timestamp and snapshot, based on the keys held by the server, a new one // might be generated and signed due to expiry of the previous one or updates // to other roles. -func getMaybeServerSigned(ctx context.Context, w io.Writer, store storage.MetaStore, gun, role string) error { +func getMaybeServerSigned(ctx context.Context, store storage.MetaStore, gun, role string) (*time.Time, []byte, error) { cryptoServiceVal := ctx.Value("cryptoService") cryptoService, ok := cryptoServiceVal.(signed.CryptoService) if !ok { - return errors.ErrNoCryptoService.WithDetail(nil) + return nil, nil, errors.ErrNoCryptoService.WithDetail(nil) } var ( - out []byte - err error + creation *time.Time + out []byte + err error ) switch role { case data.CanonicalSnapshotRole: - out, err = snapshot.GetOrCreateSnapshot(gun, store, cryptoService) + creation, out, err = snapshot.GetOrCreateSnapshot(gun, store, cryptoService) case data.CanonicalTimestampRole: - out, err = timestamp.GetOrCreateTimestamp(gun, store, cryptoService) + creation, out, err = timestamp.GetOrCreateTimestamp(gun, store, cryptoService) } if err != nil { switch err.(type) { case *storage.ErrNoKey, storage.ErrNotFound: - return errors.ErrMetadataNotFound.WithDetail(err) + return nil, nil, errors.ErrMetadataNotFound.WithDetail(err) default: - return errors.ErrUnknown.WithDetail(err) + return nil, nil, errors.ErrUnknown.WithDetail(err) } } - w.Write(out) - return nil + return creation, out, nil } diff --git a/server/handlers/roles_test.go b/server/handlers/roles_test.go index ce6df0d4c5..3c12d292d7 100644 --- a/server/handlers/roles_test.go +++ b/server/handlers/roles_test.go @@ -14,10 +14,7 @@ import ( ) func TestGetMaybeServerSignedNoCrypto(t *testing.T) { - err := getMaybeServerSigned( - context.Background(), - nil, nil, "", "", - ) + _, _, err := getMaybeServerSigned(context.Background(), nil, "", "") require.Error(t, err) require.IsType(t, errcode.Error{}, err) @@ -33,9 +30,8 @@ func TestGetMaybeServerSignedNoKey(t *testing.T) { ctx = context.WithValue(ctx, "cryptoService", crypto) ctx = context.WithValue(ctx, "keyAlgorithm", data.ED25519Key) - err := getMaybeServerSigned( + _, _, err := getMaybeServerSigned( ctx, - nil, store, "gun", data.CanonicalTimestampRole, diff --git a/server/snapshot/snapshot.go b/server/snapshot/snapshot.go index 9492e2fd0d..be31c6fc8c 100644 --- a/server/snapshot/snapshot.go +++ b/server/snapshot/snapshot.go @@ -2,6 +2,7 @@ package snapshot import ( "encoding/json" + "time" "github.com/Sirupsen/logrus" @@ -45,10 +46,12 @@ func GetOrCreateSnapshotKey(gun string, store storage.KeyStore, crypto signed.Cr // GetOrCreateSnapshot either returns the exisiting latest snapshot, or uses // whatever the most recent snapshot is to create the next one, only updating // the expiry time and version. -func GetOrCreateSnapshot(gun string, store storage.MetaStore, cryptoService signed.CryptoService) ([]byte, error) { - _, d, err := store.GetCurrent(gun, data.CanonicalSnapshotRole) +func GetOrCreateSnapshot(gun string, store storage.MetaStore, cryptoService signed.CryptoService) ( + *time.Time, []byte, error) { + + creation, d, err := store.GetCurrent(gun, data.CanonicalSnapshotRole) if err != nil { - return nil, err + return nil, nil, err } sn := &data.SignedSnapshot{} @@ -56,29 +59,30 @@ func GetOrCreateSnapshot(gun string, store storage.MetaStore, cryptoService sign err := json.Unmarshal(d, sn) if err != nil { logrus.Error("Failed to unmarshal existing snapshot") - return nil, err + return nil, nil, err } if !snapshotExpired(sn) { - return d, nil + return creation, d, nil } } sgnd, version, err := createSnapshot(gun, sn, store, cryptoService) if err != nil { logrus.Error("Failed to create a new snapshot") - return nil, err + return nil, nil, err } out, err := json.Marshal(sgnd) if err != nil { logrus.Error("Failed to marshal new snapshot") - return nil, err + return nil, nil, err } err = store.UpdateCurrent(gun, storage.MetaUpdate{Role: "snapshot", Version: version, Data: out}) if err != nil { - return nil, err + return nil, nil, err } - return out, nil + c := time.Now() + return &c, out, nil } // snapshotExpired simply checks if the snapshot is past its expiry time diff --git a/server/snapshot/snapshot_test.go b/server/snapshot/snapshot_test.go index b2226b5c8f..6be143ae32 100644 --- a/server/snapshot/snapshot_test.go +++ b/server/snapshot/snapshot_test.go @@ -118,7 +118,7 @@ func TestGetSnapshotNotExists(t *testing.T) { store := storage.NewMemStorage() crypto := signed.NewEd25519() - _, err := GetOrCreateSnapshot("gun", store, crypto) + _, _, err := GetOrCreateSnapshot("gun", store, crypto) assert.Error(t, err) } @@ -144,18 +144,23 @@ func TestGetSnapshotCurrValid(t *testing.T) { // test when db is missing the role data store.UpdateCurrent("gun", storage.MetaUpdate{Role: "snapshot", Version: 0, Data: snapJSON}) - _, err = GetOrCreateSnapshot("gun", store, crypto) + c1, result, err := GetOrCreateSnapshot("gun", store, crypto) assert.NoError(t, err) + assert.True(t, bytes.Equal(snapJSON, result)) // test when db has the role data store.UpdateCurrent("gun", storage.MetaUpdate{Role: "root", Version: 0, Data: newData}) - _, err = GetOrCreateSnapshot("gun", store, crypto) + c2, result, err := GetOrCreateSnapshot("gun", store, crypto) assert.NoError(t, err) + assert.True(t, bytes.Equal(snapJSON, result)) + assert.True(t, c1.Equal(*c2)) - // test when db role data is expired + // test when db role data is corrupt store.UpdateCurrent("gun", storage.MetaUpdate{Role: "root", Version: 1, Data: []byte{3}}) - _, err = GetOrCreateSnapshot("gun", store, crypto) + c2, result, err = GetOrCreateSnapshot("gun", store, crypto) assert.NoError(t, err) + assert.True(t, bytes.Equal(snapJSON, result)) + assert.True(t, c1.Equal(*c2)) } func TestGetSnapshotCurrExpired(t *testing.T) { @@ -168,8 +173,10 @@ func TestGetSnapshotCurrExpired(t *testing.T) { snapJSON, _ := json.Marshal(snapshot) store.UpdateCurrent("gun", storage.MetaUpdate{Role: "snapshot", Version: 0, Data: snapJSON}) - _, err = GetOrCreateSnapshot("gun", store, crypto) + c1, newJSON, err := GetOrCreateSnapshot("gun", store, crypto) assert.NoError(t, err) + assert.False(t, bytes.Equal(snapJSON, newJSON)) + assert.True(t, c1.After(time.Now().Add(-1*time.Minute))) } func TestGetSnapshotCurrCorrupt(t *testing.T) { @@ -182,7 +189,7 @@ func TestGetSnapshotCurrCorrupt(t *testing.T) { snapJSON, _ := json.Marshal(snapshot) store.UpdateCurrent("gun", storage.MetaUpdate{Role: "snapshot", Version: 0, Data: snapJSON[1:]}) - _, err = GetOrCreateSnapshot("gun", store, crypto) + _, _, err = GetOrCreateSnapshot("gun", store, crypto) assert.Error(t, err) } diff --git a/server/timestamp/timestamp.go b/server/timestamp/timestamp.go index 645c27eea4..5b1555deea 100644 --- a/server/timestamp/timestamp.go +++ b/server/timestamp/timestamp.go @@ -1,6 +1,8 @@ package timestamp import ( + "time" + "github.com/docker/go/canonical/json" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/signed" @@ -47,16 +49,18 @@ func GetOrCreateTimestampKey(gun string, store storage.MetaStore, crypto signed. // GetOrCreateTimestamp returns the current timestamp for the gun. This may mean // a new timestamp is generated either because none exists, or because the current // one has expired. Once generated, the timestamp is saved in the store. -func GetOrCreateTimestamp(gun string, store storage.MetaStore, cryptoService signed.CryptoService) ([]byte, error) { - snapshot, err := snapshot.GetOrCreateSnapshot(gun, store, cryptoService) +func GetOrCreateTimestamp(gun string, store storage.MetaStore, cryptoService signed.CryptoService) ( + *time.Time, []byte, error) { + + _, snapshot, err := snapshot.GetOrCreateSnapshot(gun, store, cryptoService) if err != nil { - return nil, err + return nil, nil, err } - _, d, err := store.GetCurrent(gun, "timestamp") + creation, d, err := store.GetCurrent(gun, data.CanonicalTimestampRole) if err != nil { if _, ok := err.(storage.ErrNotFound); !ok { logrus.Error("error retrieving timestamp: ", err.Error()) - return nil, err + return nil, nil, err } logrus.Debug("No timestamp found, will proceed to create first timestamp") } @@ -65,27 +69,28 @@ func GetOrCreateTimestamp(gun string, store storage.MetaStore, cryptoService sig err := json.Unmarshal(d, ts) if err != nil { logrus.Error("Failed to unmarshal existing timestamp") - return nil, err + return nil, nil, err } if !timestampExpired(ts) && !snapshotExpired(ts, snapshot) { - return d, nil + return creation, d, nil } } sgnd, version, err := CreateTimestamp(gun, ts, snapshot, store, cryptoService) if err != nil { logrus.Error("Failed to create a new timestamp") - return nil, err + return nil, nil, err } out, err := json.Marshal(sgnd) if err != nil { logrus.Error("Failed to marshal new timestamp") - return nil, err + return nil, nil, err } err = store.UpdateCurrent(gun, storage.MetaUpdate{Role: "timestamp", Version: version, Data: out}) if err != nil { - return nil, err + return nil, nil, err } - return out, nil + c := time.Now() + return &c, out, nil } // timestampExpired compares the current time to the expiry time of the timestamp diff --git a/server/timestamp/timestamp_test.go b/server/timestamp/timestamp_test.go index fe67456e10..2ec28b59b6 100644 --- a/server/timestamp/timestamp_test.go +++ b/server/timestamp/timestamp_test.go @@ -64,7 +64,7 @@ func TestGetTimestamp(t *testing.T) { _, err := GetOrCreateTimestampKey("gun", store, crypto, data.ED25519Key) assert.Nil(t, err, "GetKey errored") - _, err = GetOrCreateTimestamp("gun", store, crypto) + _, _, err = GetOrCreateTimestamp("gun", store, crypto) assert.Nil(t, err, "GetTimestamp errored") } @@ -85,7 +85,7 @@ func TestGetTimestampNewSnapshot(t *testing.T) { _, err := GetOrCreateTimestampKey("gun", store, crypto, data.ED25519Key) assert.Nil(t, err, "GetKey errored") - ts1, err := GetOrCreateTimestamp("gun", store, crypto) + c1, ts1, err := GetOrCreateTimestamp("gun", store, crypto) assert.Nil(t, err, "GetTimestamp errored") snapshot = &data.SignedSnapshot{ @@ -98,8 +98,8 @@ func TestGetTimestampNewSnapshot(t *testing.T) { store.UpdateCurrent("gun", storage.MetaUpdate{Role: "snapshot", Version: 1, Data: snapJSON}) - ts2, err := GetOrCreateTimestamp("gun", store, crypto) + c2, ts2, err := GetOrCreateTimestamp("gun", store, crypto) assert.NoError(t, err, "GetTimestamp errored") - assert.NotEqual(t, ts1, ts2, "Timestamp was not regenerated when snapshot changed") + assert.True(t, c1.Before(*c2), "Timestamp creation time incorrect") }