diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index f13bdc476e..bd686a7b42 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -66,26 +66,23 @@ func createGitHubClients(cfg github.MCPServerConfig, apiHost utils.APIHostResolv // authenticate via BearerAuthTransport and skip go-github's WithAuthToken: // the latter installs its own round tripper that would pin the static token // and shadow the dynamic one. + // + // ETagTransport sits below the user-agent (and auth) layers so that, by the + // time it runs, the Authorization header is set and can scope the + // conditional-request cache per token. It adds ETag/If-None-Match handling + // so unchanged resources are revalidated with a 304 instead of being + // re-downloaded in full. + // + // The conditional-request cache is enabled only for the REST API client on + // this long-lived local (stdio) server. The raw-content client below uses a + // separate transport without it, so large file bodies are never buffered + // into the cache. The hosted, horizontally-scaled server builds a fresh REST + // client per request (see pkg/github RequestDeps) and does not use this path. restUATransport := &transport.UserAgentTransport{ - Transport: http.DefaultTransport, + Transport: &transport.ETagTransport{Transport: http.DefaultTransport}, Agent: fmt.Sprintf("github-mcp-server/%s", cfg.Version), } - var restClient *gogithub.Client - if cfg.TokenProvider != nil { - restClient, err = gogithub.NewClient( - gogithub.WithHTTPClient(&http.Client{Transport: &transport.BearerAuthTransport{ - Transport: restUATransport, - TokenProvider: cfg.TokenProvider, - }}), - gogithub.WithEnterpriseURLs(restURL.String(), uploadURL.String()), - ) - } else { - restClient, err = gogithub.NewClient( - gogithub.WithHTTPClient(&http.Client{Transport: restUATransport}), - gogithub.WithAuthToken(cfg.Token), - gogithub.WithEnterpriseURLs(restURL.String(), uploadURL.String()), - ) - } + restClient, err := newRESTClient(cfg, restUATransport, restURL.String(), uploadURL.String()) if err != nil { return nil, fmt.Errorf("failed to create REST client: %w", err) } @@ -104,8 +101,18 @@ func createGitHubClients(cfg github.MCPServerConfig, apiHost utils.APIHostResolv gqlClient := githubv4.NewEnterpriseClient(graphQLURL.String(), gqlHTTPClient) - // Create raw content client (shares REST client's HTTP transport) - rawClient, err := raw.NewClient(restClient, rawURL) + // Create raw content client. It shares the REST client's authentication but + // uses a transport without the conditional-request cache: raw file bodies can + // be large and are streamed rather than retained in memory. + rawUATransport := &transport.UserAgentTransport{ + Transport: http.DefaultTransport, + Agent: fmt.Sprintf("github-mcp-server/%s", cfg.Version), + } + rawRESTClient, err := newRESTClient(cfg, rawUATransport, restURL.String(), uploadURL.String()) + if err != nil { + return nil, fmt.Errorf("failed to create raw REST client: %w", err) + } + rawClient, err := raw.NewClient(rawRESTClient, rawURL) if err != nil { return nil, fmt.Errorf("failed to create raw client: %w", err) } @@ -132,6 +139,27 @@ func createGitHubClients(cfg github.MCPServerConfig, apiHost utils.APIHostResolv }, nil } +// newRESTClient builds a go-github REST client that sends requests through the +// supplied user-agent transport, wiring authentication to match the server +// configuration: a dynamic TokenProvider via BearerAuthTransport, or a static +// token via go-github's WithAuthToken. +func newRESTClient(cfg github.MCPServerConfig, uaTransport *transport.UserAgentTransport, restURL, uploadURL string) (*gogithub.Client, error) { + if cfg.TokenProvider != nil { + return gogithub.NewClient( + gogithub.WithHTTPClient(&http.Client{Transport: &transport.BearerAuthTransport{ + Transport: uaTransport, + TokenProvider: cfg.TokenProvider, + }}), + gogithub.WithEnterpriseURLs(restURL, uploadURL), + ) + } + return gogithub.NewClient( + gogithub.WithHTTPClient(&http.Client{Transport: uaTransport}), + gogithub.WithAuthToken(cfg.Token), + gogithub.WithEnterpriseURLs(restURL, uploadURL), + ) +} + func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Server, error) { apiHost, err := utils.NewAPIHost(cfg.Host) if err != nil { diff --git a/pkg/http/headers/headers.go b/pkg/http/headers/headers.go index e032a0ce93..0ac24e5471 100644 --- a/pkg/http/headers/headers.go +++ b/pkg/http/headers/headers.go @@ -9,6 +9,14 @@ const ( AcceptHeader = "Accept" // UserAgentHeader is a standard HTTP Header. UserAgentHeader = "User-Agent" + // ETagHeader is a standard HTTP Header carrying a response entity tag. + ETagHeader = "ETag" + // IfNoneMatchHeader is a standard HTTP Header used to make a request conditional on an entity tag. + IfNoneMatchHeader = "If-None-Match" + // CacheControlHeader is a standard HTTP Header carrying caching directives. + CacheControlHeader = "Cache-Control" + // VaryHeader is a standard HTTP Header describing which request headers a response varies on. + VaryHeader = "Vary" // ContentTypeJSON is the standard MIME type for JSON. ContentTypeJSON = "application/json" diff --git a/pkg/http/transport/etag.go b/pkg/http/transport/etag.go new file mode 100644 index 0000000000..827b787efd --- /dev/null +++ b/pkg/http/transport/etag.go @@ -0,0 +1,314 @@ +package transport + +import ( + "bytes" + "container/list" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "net/http" + "strings" + "sync" + + "github.com/github/github-mcp-server/pkg/http/headers" +) + +const ( + // defaultETagCacheSize bounds the number of cached conditional responses held + // in memory by an ETagTransport. + defaultETagCacheSize = 512 + + // defaultMaxEntryBytes bounds the size of a single cached body. Responses + // larger than this are still revalidated normally but are never retained, so + // a few large reads cannot pin large buffers in memory. + defaultMaxEntryBytes = 1 << 20 // 1 MiB + + // defaultMaxTotalBytes bounds the combined size of all cached bodies. The + // least-recently-used entries are evicted until the cache is within budget. + defaultMaxTotalBytes = 32 << 20 // 32 MiB +) + +// rateLimitHeaders are copied from the live 304 response onto a cache-served +// response so downstream rate-limit accounting observes the current state. +var rateLimitHeaders = []string{ + "X-RateLimit-Limit", + "X-RateLimit-Remaining", + "X-RateLimit-Used", + "X-RateLimit-Reset", + "X-RateLimit-Resource", + "Retry-After", + "Date", +} + +// etagEntry is a cached response body and headers keyed by an ETag. +type etagEntry struct { + etag string + status int + header http.Header + body []byte +} + +// response reconstructs an *http.Response from a cached entry, layering the +// live 304 response's rate-limit and timing headers on top so the caller sees +// the current rate-limit state while receiving the cached body. +func (e etagEntry) response(live *http.Response) *http.Response { + h := e.header.Clone() + for _, name := range rateLimitHeaders { + if values := live.Header.Values(name); len(values) > 0 { + h.Del(name) + for _, v := range values { + h.Add(name, v) + } + } + } + return &http.Response{ + Status: fmt.Sprintf("%d %s", e.status, http.StatusText(e.status)), + StatusCode: e.status, + Proto: live.Proto, + ProtoMajor: live.ProtoMajor, + ProtoMinor: live.ProtoMinor, + Header: h, + Body: io.NopCloser(bytes.NewReader(e.body)), + ContentLength: int64(len(e.body)), + Request: live.Request, + } +} + +type lruItem struct { + key string + entry etagEntry +} + +// ETagTransport is an http.RoundTripper that adds HTTP conditional-request +// support (ETag / If-None-Match) to GET requests. For each cacheable GET it +// stores the response ETag and body; on a subsequent identical request it sends +// If-None-Match and, when the server answers 304 Not Modified, serves the +// cached body instead of re-downloading it. +// +// Every request is still sent to the server, so responses are always +// revalidated and never served stale. A 304 Not Modified does not count against +// the GitHub REST API primary rate limit, so revalidated requests conserve +// rate-limit budget and bandwidth. +// +// Cached entries are scoped by the request's Authorization header so responses +// are never shared across tokens. Responses marked non-storable by HTTP cache +// directives (request or response Cache-Control: no-store, or Vary: *) are never +// retained. The cache is bounded both by entry count and by a total-byte budget +// (LRU) and is safe for concurrent use. +// +// This transport is intended for the long-lived local (stdio) server only. The +// hosted, horizontally-scaled server constructs a fresh REST client per request +// and does not use it, so an in-process cache adds nothing there. +type ETagTransport struct { + Transport http.RoundTripper + + // MaxEntries bounds the number of cached responses. When zero, + // defaultETagCacheSize is used. + MaxEntries int + + // MaxEntryBytes bounds the size of a single cached body. Responses larger + // than this are revalidated but not retained. When zero, defaultMaxEntryBytes + // is used. + MaxEntryBytes int + + // MaxTotalBytes bounds the combined size of all cached bodies. When zero, + // defaultMaxTotalBytes is used. + MaxTotalBytes int + + mu sync.Mutex + ll *list.List + items map[string]*list.Element + curBytes int +} + +func (t *ETagTransport) RoundTrip(req *http.Request) (*http.Response, error) { + rt := t.Transport + if rt == nil { + rt = http.DefaultTransport + } + + // Only cache GET requests, never override a caller-supplied conditional + // header, and honor a client request to bypass the cache entirely. + if req.Method != http.MethodGet || req.Header.Get(headers.IfNoneMatchHeader) != "" || hasNoStore(req.Header) { + return rt.RoundTrip(req) + } + + key := cacheKey(req) + cached, ok := t.get(key) + + req = req.Clone(req.Context()) + if ok { + req.Header.Set(headers.IfNoneMatchHeader, cached.etag) + } + + resp, err := rt.RoundTrip(req) + if err != nil { + return resp, err + } + + if resp.StatusCode == http.StatusNotModified && ok { + // Discard the empty 304 body and serve the cached response instead. + if resp.Body != nil { + _, _ = io.Copy(io.Discard, resp.Body) + resp.Body.Close() + } + return cached.response(resp), nil + } + + if resp.StatusCode == http.StatusOK { + etag := resp.Header.Get(headers.ETagHeader) + + // Drop any prior entry and skip caching when the response is missing an + // ETag or is marked non-storable by HTTP cache directives. + if etag == "" || !storable(resp) { + t.remove(key) + return resp, nil + } + + // Skip caching bodies that exceed the per-entry byte budget. When the + // length is known up front, avoid buffering the body at all. + maxEntry := t.maxEntryBytes() + if resp.ContentLength > int64(maxEntry) { + t.remove(key) + return resp, nil + } + + body, readErr := io.ReadAll(resp.Body) + resp.Body.Close() + if readErr != nil { + return nil, readErr + } + resp.Body = io.NopCloser(bytes.NewReader(body)) + resp.ContentLength = int64(len(body)) + + if len(body) > maxEntry { + t.remove(key) + return resp, nil + } + + t.add(key, etagEntry{ + etag: etag, + status: resp.StatusCode, + header: resp.Header.Clone(), + body: body, + }) + } + + return resp, nil +} + +// hasNoStore reports whether a Cache-Control header carries the no-store +// directive. +func hasNoStore(h http.Header) bool { + for _, cc := range h.Values(headers.CacheControlHeader) { + for _, directive := range strings.Split(cc, ",") { + if strings.EqualFold(strings.TrimSpace(directive), "no-store") { + return true + } + } + } + return false +} + +// storable reports whether a response may be retained. Responses that request +// no-store or that vary on every request (Vary: *) must not be cached. +func storable(resp *http.Response) bool { + if hasNoStore(resp.Header) { + return false + } + for _, vary := range resp.Header.Values(headers.VaryHeader) { + for _, field := range strings.Split(vary, ",") { + if strings.TrimSpace(field) == "*" { + return false + } + } + } + return true +} + +func cacheKey(req *http.Request) string { + sum := sha256.Sum256([]byte(req.Header.Get(headers.AuthorizationHeader))) + return req.Method + " " + req.URL.String() + " " + hex.EncodeToString(sum[:8]) +} + +func (t *ETagTransport) get(key string) (etagEntry, bool) { + t.mu.Lock() + defer t.mu.Unlock() + if t.items == nil { + return etagEntry{}, false + } + el, ok := t.items[key] + if !ok { + return etagEntry{}, false + } + t.ll.MoveToFront(el) + return el.Value.(*lruItem).entry, true +} + +func (t *ETagTransport) maxEntryBytes() int { + if t.MaxEntryBytes > 0 { + return t.MaxEntryBytes + } + return defaultMaxEntryBytes +} + +func (t *ETagTransport) maxTotalBytes() int { + if t.MaxTotalBytes > 0 { + return t.MaxTotalBytes + } + return defaultMaxTotalBytes +} + +// remove drops a cached entry if present, keeping the byte accounting in sync. +func (t *ETagTransport) remove(key string) { + t.mu.Lock() + defer t.mu.Unlock() + if t.items == nil { + return + } + el, ok := t.items[key] + if !ok { + return + } + t.curBytes -= len(el.Value.(*lruItem).entry.body) + t.ll.Remove(el) + delete(t.items, key) +} + +func (t *ETagTransport) add(key string, entry etagEntry) { + t.mu.Lock() + defer t.mu.Unlock() + if t.items == nil { + t.items = make(map[string]*list.Element) + t.ll = list.New() + } + if el, ok := t.items[key]; ok { + item := el.Value.(*lruItem) + t.curBytes += len(entry.body) - len(item.entry.body) + item.entry = entry + t.ll.MoveToFront(el) + } else { + el := t.ll.PushFront(&lruItem{key: key, entry: entry}) + t.items[key] = el + t.curBytes += len(entry.body) + } + + limit := t.MaxEntries + if limit <= 0 { + limit = defaultETagCacheSize + } + maxBytes := t.maxTotalBytes() + // Evict least-recently-used entries until within both the entry-count and + // total-byte budgets. Always keep at least the entry just inserted. + for t.ll.Len() > 1 && (t.ll.Len() > limit || t.curBytes > maxBytes) { + oldest := t.ll.Back() + if oldest == nil { + break + } + item := oldest.Value.(*lruItem) + t.curBytes -= len(item.entry.body) + t.ll.Remove(oldest) + delete(t.items, item.key) + } +} diff --git a/pkg/http/transport/etag_test.go b/pkg/http/transport/etag_test.go new file mode 100644 index 0000000000..ebbc222dee --- /dev/null +++ b/pkg/http/transport/etag_test.go @@ -0,0 +1,375 @@ +package transport + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/github/github-mcp-server/pkg/http/headers" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestETagTransport_ServesCachedBodyOn304 verifies the core conditional-request +// flow: the first GET carries no If-None-Match and is cached with its ETag; the +// second GET sends the cached ETag and, on a 304 Not Modified, is served the +// cached body instead of the empty 304 body. +func TestETagTransport_ServesCachedBodyOn304(t *testing.T) { + t.Parallel() + + const etag = `"abc123"` + const body = `{"number":1}` + + var requests int32 + var lastIfNoneMatch string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + n := atomic.AddInt32(&requests, 1) + lastIfNoneMatch = r.Header.Get(headers.IfNoneMatchHeader) + w.Header().Set(headers.ETagHeader, etag) + if n == 1 { + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, body) + return + } + // Second request revalidates and is unchanged. + w.WriteHeader(http.StatusNotModified) + })) + defer server.Close() + + rt := &ETagTransport{Transport: http.DefaultTransport} + + do := func() (int, string) { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + require.NoError(t, err) + resp, err := rt.RoundTrip(req) + require.NoError(t, err) + defer resp.Body.Close() + data, err := io.ReadAll(resp.Body) + require.NoError(t, err) + return resp.StatusCode, string(data) + } + + status1, body1 := do() + assert.Equal(t, http.StatusOK, status1) + assert.Equal(t, body, body1) + assert.Empty(t, lastIfNoneMatch, "first request must not send If-None-Match") + + status2, body2 := do() + assert.Equal(t, http.StatusOK, status2, "304 is translated to the cached 200") + assert.Equal(t, body, body2, "cached body is served on 304") + assert.Equal(t, etag, lastIfNoneMatch, "second request sends the cached ETag") + assert.Equal(t, int32(2), atomic.LoadInt32(&requests), "every request still reaches the server") +} + +// TestETagTransport_UpdatesRateLimitHeadersFrom304 verifies that a cache-served +// response surfaces the live 304 response's rate-limit headers rather than the +// stale headers captured with the cached body. +func TestETagTransport_UpdatesRateLimitHeadersFrom304(t *testing.T) { + t.Parallel() + + const etag = `"v1"` + + var requests int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + n := atomic.AddInt32(&requests, 1) + w.Header().Set(headers.ETagHeader, etag) + if n == 1 { + w.Header().Set("X-RateLimit-Remaining", "100") + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, "cached") + return + } + w.Header().Set("X-RateLimit-Remaining", "99") + w.WriteHeader(http.StatusNotModified) + })) + defer server.Close() + + rt := &ETagTransport{Transport: http.DefaultTransport} + + do := func() http.Header { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + require.NoError(t, err) + resp, err := rt.RoundTrip(req) + require.NoError(t, err) + _, _ = io.Copy(io.Discard, resp.Body) + resp.Body.Close() + return resp.Header + } + + h1 := do() + assert.Equal(t, "100", h1.Get("X-RateLimit-Remaining")) + + h2 := do() + assert.Equal(t, "99", h2.Get("X-RateLimit-Remaining"), "rate-limit headers come from the live 304") +} + +// TestETagTransport_ScopesCacheByAuthorization verifies cached bodies are not +// shared across tokens. +func TestETagTransport_ScopesCacheByAuthorization(t *testing.T) { + t.Parallel() + + const etag = `"shared-url"` + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set(headers.ETagHeader, etag) + if r.Header.Get(headers.IfNoneMatchHeader) != "" { + w.WriteHeader(http.StatusNotModified) + return + } + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, r.Header.Get(headers.AuthorizationHeader)) + })) + defer server.Close() + + rt := &ETagTransport{Transport: http.DefaultTransport} + + get := func(auth string) string { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + require.NoError(t, err) + req.Header.Set(headers.AuthorizationHeader, auth) + resp, err := rt.RoundTrip(req) + require.NoError(t, err) + defer resp.Body.Close() + data, err := io.ReadAll(resp.Body) + require.NoError(t, err) + return string(data) + } + + assert.Equal(t, "Bearer a", get("Bearer a")) + assert.Equal(t, "Bearer b", get("Bearer b"), "a different token must not receive the other token's cached body") + assert.Equal(t, "Bearer a", get("Bearer a"), "the first token's cached body is served on revalidation") +} + +// TestETagTransport_OnlyCachesGET verifies non-GET requests bypass the cache and +// are never sent a conditional header. +func TestETagTransport_OnlyCachesGET(t *testing.T) { + t.Parallel() + + var sawIfNoneMatch bool + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get(headers.IfNoneMatchHeader) != "" { + sawIfNoneMatch = true + } + w.Header().Set(headers.ETagHeader, `"x"`) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + rt := &ETagTransport{Transport: http.DefaultTransport} + + do := func() { + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, server.URL, nil) + require.NoError(t, err) + resp, err := rt.RoundTrip(req) + require.NoError(t, err) + _, _ = io.Copy(io.Discard, resp.Body) + resp.Body.Close() + } + + do() + do() + assert.False(t, sawIfNoneMatch, "POST requests must not be revalidated") +} + +// TestETagTransport_DoesNotOverrideCallerConditional verifies a caller-supplied +// If-None-Match header is preserved and not replaced by the cache. +func TestETagTransport_DoesNotOverrideCallerConditional(t *testing.T) { + t.Parallel() + + var gotIfNoneMatch string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotIfNoneMatch = r.Header.Get(headers.IfNoneMatchHeader) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + rt := &ETagTransport{Transport: http.DefaultTransport} + + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + require.NoError(t, err) + req.Header.Set(headers.IfNoneMatchHeader, `"caller"`) + + resp, err := rt.RoundTrip(req) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, `"caller"`, gotIfNoneMatch) +} + +// TestETagTransport_SkipsBodiesOverEntryByteBudget verifies a response whose +// body exceeds the per-entry byte budget is served but never cached, so a +// subsequent request is not revalidated against a stored ETag. +func TestETagTransport_SkipsBodiesOverEntryByteBudget(t *testing.T) { + t.Parallel() + + const etag = `"big"` + body := make([]byte, 64) + + var lastIfNoneMatch string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + lastIfNoneMatch = r.Header.Get(headers.IfNoneMatchHeader) + w.Header().Set(headers.ETagHeader, etag) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(body) + })) + defer server.Close() + + rt := &ETagTransport{Transport: http.DefaultTransport, MaxEntryBytes: 16} + + do := func() []byte { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + require.NoError(t, err) + resp, err := rt.RoundTrip(req) + require.NoError(t, err) + defer resp.Body.Close() + data, err := io.ReadAll(resp.Body) + require.NoError(t, err) + return data + } + + assert.Equal(t, body, do()) + assert.Equal(t, body, do(), "oversized body is served in full each time") + assert.Empty(t, lastIfNoneMatch, "an oversized response must not be cached or revalidated") +} + +// TestETagTransport_EvictsByTotalByteBudget verifies that inserting a second +// entry that pushes the cache over its total-byte budget evicts the +// least-recently-used entry, which is then re-fetched in full. +func TestETagTransport_EvictsByTotalByteBudget(t *testing.T) { + t.Parallel() + + body := make([]byte, 10) + ifNoneMatch := map[string]string{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ifNoneMatch[r.URL.Path] = r.Header.Get(headers.IfNoneMatchHeader) + w.Header().Set(headers.ETagHeader, `"`+r.URL.Path+`"`) + if r.Header.Get(headers.IfNoneMatchHeader) != "" { + w.WriteHeader(http.StatusNotModified) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write(body) + })) + defer server.Close() + + // Budget holds a single 10-byte entry; a second entry evicts the first. + rt := &ETagTransport{Transport: http.DefaultTransport, MaxTotalBytes: 15} + + get := func(path string) { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL+path, nil) + require.NoError(t, err) + resp, err := rt.RoundTrip(req) + require.NoError(t, err) + _, _ = io.Copy(io.Discard, resp.Body) + resp.Body.Close() + } + + get("/a") // cache A + get("/b") // cache B, evicts A + get("/a") // A was evicted -> full GET, no conditional header + + assert.Empty(t, ifNoneMatch["/a"], "A must be re-fetched in full after eviction") +} + +// TestETagTransport_DoesNotCacheNoStoreResponse verifies a response marked +// Cache-Control: no-store is never retained. +func TestETagTransport_DoesNotCacheNoStoreResponse(t *testing.T) { + t.Parallel() + + var lastIfNoneMatch string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + lastIfNoneMatch = r.Header.Get(headers.IfNoneMatchHeader) + w.Header().Set(headers.ETagHeader, `"ns"`) + w.Header().Set(headers.CacheControlHeader, "private, no-store") + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, "secret") + })) + defer server.Close() + + rt := &ETagTransport{Transport: http.DefaultTransport} + + do := func() { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + require.NoError(t, err) + resp, err := rt.RoundTrip(req) + require.NoError(t, err) + _, _ = io.Copy(io.Discard, resp.Body) + resp.Body.Close() + } + + do() + do() + assert.Empty(t, lastIfNoneMatch, "a no-store response must not be cached or revalidated") +} + +// TestETagTransport_DoesNotCacheVaryStar verifies a response with Vary: * is +// never retained. +func TestETagTransport_DoesNotCacheVaryStar(t *testing.T) { + t.Parallel() + + var lastIfNoneMatch string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + lastIfNoneMatch = r.Header.Get(headers.IfNoneMatchHeader) + w.Header().Set(headers.ETagHeader, `"vary"`) + w.Header().Set(headers.VaryHeader, "*") + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, "body") + })) + defer server.Close() + + rt := &ETagTransport{Transport: http.DefaultTransport} + + do := func() { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + require.NoError(t, err) + resp, err := rt.RoundTrip(req) + require.NoError(t, err) + _, _ = io.Copy(io.Discard, resp.Body) + resp.Body.Close() + } + + do() + do() + assert.Empty(t, lastIfNoneMatch, "a Vary:* response must not be cached or revalidated") +} + +// TestETagTransport_RequestNoStoreBypassesCache verifies a client request that +// sends Cache-Control: no-store neither reads from nor revalidates against the +// cache, even after a prior response was cached. +func TestETagTransport_RequestNoStoreBypassesCache(t *testing.T) { + t.Parallel() + + var lastIfNoneMatch string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + lastIfNoneMatch = r.Header.Get(headers.IfNoneMatchHeader) + w.Header().Set(headers.ETagHeader, `"x"`) + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, "body") + })) + defer server.Close() + + rt := &ETagTransport{Transport: http.DefaultTransport} + + // Prime the cache with a normal GET. + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + require.NoError(t, err) + resp, err := rt.RoundTrip(req) + require.NoError(t, err) + _, _ = io.Copy(io.Discard, resp.Body) + resp.Body.Close() + + // A no-store request must not send If-None-Match. + req2, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + require.NoError(t, err) + req2.Header.Set(headers.CacheControlHeader, "no-store") + resp2, err := rt.RoundTrip(req2) + require.NoError(t, err) + _, _ = io.Copy(io.Discard, resp2.Body) + resp2.Body.Close() + + assert.Empty(t, lastIfNoneMatch, "a no-store request must bypass the cache") +}