Skip to content

Commit 38842a6

Browse files
zharinovzharinov
authored
fix(http): Cache HEAD requests separately from GET requests (#37862)
1 parent 6435cf3 commit 38842a6

9 files changed

+284
-36
lines changed

‎lib/util/cache/repository/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ export interface BranchCache {
128128
export interface RepoCacheData {
129129
configFileName?: string;
130130
httpCache?: Record<string, unknown>;
131+
httpCacheHead?: Record<string, unknown>;
131132
semanticCommits?: 'enabled' | 'disabled';
132133
branches?: BranchCache[];
133134
init?: RepoInitConfig;

‎lib/util/http/cache/abstract-http-cache-provider.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,15 @@ import { HttpCache } from './schema';
66
import type { HttpCacheProvider } from './types';
77

88
export abstract class AbstractHttpCacheProvider implements HttpCacheProvider {
9-
protected abstract load(url: string): Promise<unknown>;
10-
protected abstract persist(url: string, data: HttpCache): Promise<void>;
9+
protected abstract load(method: string, url: string): Promise<unknown>;
10+
protected abstract persist(
11+
method: string,
12+
url: string,
13+
data: HttpCache,
14+
): Promise<void>;
1115

12-
async get(url: string): Promise<HttpCache | null> {
13-
const cache = await this.load(url);
16+
async get(method: string, url: string): Promise<HttpCache | null> {
17+
const cache = await this.load(method, url);
1418
const httpCache = HttpCache.parse(cache);
1519
if (!httpCache) {
1620
return null;
@@ -20,10 +24,11 @@ export abstract class AbstractHttpCacheProvider implements HttpCacheProvider {
2024
}
2125

2226
async setCacheHeaders<T extends Pick<GotOptions, 'headers'>>(
27+
method: string,
2328
url: string,
2429
opts: T,
2530
): Promise<void> {
26-
const httpCache = await this.get(url);
31+
const httpCache = await this.get(method, url);
2732
if (!httpCache) {
2833
return;
2934
}
@@ -40,13 +45,15 @@ export abstract class AbstractHttpCacheProvider implements HttpCacheProvider {
4045
}
4146

4247
bypassServer<T>(
48+
_method: string,
4349
_url: string,
4450
_ignoreSoftTtl: boolean,
4551
): Promise<HttpResponse<T> | null> {
4652
return Promise.resolve(null);
4753
}
4854

4955
async wrapServerResponse<T>(
56+
method: string,
5057
url: string,
5158
resp: HttpResponse<T>,
5259
): Promise<HttpResponse<T>> {
@@ -75,12 +82,12 @@ export abstract class AbstractHttpCacheProvider implements HttpCacheProvider {
7582
logger.debug(
7683
`http cache: saving ${url} (etag=${etag}, lastModified=${lastModified})`,
7784
);
78-
await this.persist(url, newHttpCache as HttpCache);
85+
await this.persist(method, url, newHttpCache as HttpCache);
7986
return resp;
8087
}
8188

8289
if (resp.statusCode === 304) {
83-
const httpCache = await this.get(url);
90+
const httpCache = await this.get(method, url);
8491
if (!httpCache) {
8592
return resp;
8693
}
@@ -90,7 +97,7 @@ export abstract class AbstractHttpCacheProvider implements HttpCacheProvider {
9097
`http cache: Using cached response: ${url} from ${timestamp}`,
9198
);
9299
httpCache.timestamp = new Date().toISOString();
93-
await this.persist(url, httpCache);
100+
await this.persist(method, url, httpCache);
94101

95102
HttpCacheStats.incRemoteHits(url);
96103
const cachedResp = copyResponse(

‎lib/util/http/cache/memory-http-cache-provider.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,30 @@ import { AbstractHttpCacheProvider } from './abstract-http-cache-provider';
55
import type { HttpCache } from './schema';
66

77
export class MemoryHttpCacheProvider extends AbstractHttpCacheProvider {
8-
private cacheKey(url: string): string {
9-
return `memory-cache-http-provider:${url}`;
8+
private cacheKey(method: string, url: string): string {
9+
return `memory-cache-http-provider:${method}:${url}`;
1010
}
1111

12-
protected override load(url: string): Promise<unknown> {
13-
const data = memCache.get<HttpCache>(this.cacheKey(url));
12+
protected override load(method: string, url: string): Promise<unknown> {
13+
const data = memCache.get<HttpCache>(this.cacheKey(method, url));
1414
const cloned = clone(data); // Ensures cached responses cannot be mutated
1515
return Promise.resolve(cloned);
1616
}
1717

18-
protected override persist(url: string, data: HttpCache): Promise<void> {
19-
memCache.set(this.cacheKey(url), data);
18+
protected override persist(
19+
method: string,
20+
url: string,
21+
data: HttpCache,
22+
): Promise<void> {
23+
memCache.set(this.cacheKey(method, url), data);
2024
return Promise.resolve();
2125
}
2226

23-
override async bypassServer<T>(url: string): Promise<HttpResponse<T> | null> {
24-
const cached = await this.get(url);
27+
override async bypassServer<T>(
28+
method: string,
29+
url: string,
30+
): Promise<HttpResponse<T> | null> {
31+
const cached = await this.get(method, url);
2532
if (!cached) {
2633
return null;
2734
}

‎lib/util/http/cache/package-http-cache-provider.spec.ts

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,158 @@ describe('util/http/cache/package-http-cache-provider', () => {
220220
expect(res.body).toBe('cached response');
221221
});
222222

223+
describe('HEAD requests', () => {
224+
it('handles cache miss for HEAD request', async () => {
225+
const cacheProvider = new PackageHttpCacheProvider({
226+
namespace: '_test-namespace',
227+
checkAuthorizationHeader: false,
228+
checkCacheControlHeader: false,
229+
});
230+
httpMock.scope(url).head('').reply(200, '', {
231+
etag: 'foobar',
232+
'cache-control': 'max-age=180, public',
233+
});
234+
235+
const res = await http.head(url, { cacheProvider });
236+
237+
expect(res.statusCode).toBe(200);
238+
expect(cache).toEqual({
239+
'head:http://example.com/foo/bar': {
240+
etag: 'foobar',
241+
httpResponse: {
242+
statusCode: 200,
243+
headers: expect.any(Object),
244+
body: '',
245+
},
246+
lastModified: undefined,
247+
timestamp: expect.any(String),
248+
},
249+
});
250+
});
251+
252+
it('loads cache correctly for HEAD request', async () => {
253+
mockTime('2024-06-15T00:00:00.000Z');
254+
255+
cache['head:' + url] = {
256+
etag: 'etag-value',
257+
lastModified: 'Fri, 15 Jun 2024 00:00:00 GMT',
258+
httpResponse: { statusCode: 200, body: '' },
259+
timestamp: '2024-06-15T00:00:00.000Z',
260+
};
261+
const cacheProvider = new PackageHttpCacheProvider({
262+
namespace: '_test-namespace',
263+
softTtlMinutes: 0,
264+
checkAuthorizationHeader: false,
265+
checkCacheControlHeader: false,
266+
});
267+
httpMock.scope(url).head('').reply(200, '');
268+
269+
const res = await http.head(url, { cacheProvider });
270+
271+
expect(res.statusCode).toBe(200);
272+
});
273+
274+
it('loads cache bypassing server for HEAD request', async () => {
275+
mockTime('2024-06-15T00:14:59.999Z');
276+
cache['head:' + url] = {
277+
etag: 'etag-value',
278+
lastModified: 'Fri, 15 Jun 2024 00:00:00 GMT',
279+
httpResponse: { statusCode: 200, body: '' },
280+
timestamp: '2024-06-15T00:00:00.000Z',
281+
};
282+
const cacheProvider = new PackageHttpCacheProvider({
283+
namespace: '_test-namespace',
284+
checkAuthorizationHeader: false,
285+
checkCacheControlHeader: false,
286+
});
287+
288+
const res = await http.head(url, { cacheProvider });
289+
290+
expect(res.statusCode).toBe(200);
291+
expect(packageCache.setWithRawTtl).not.toHaveBeenCalled();
292+
});
293+
294+
it('serves stale HEAD response during revalidation error', async () => {
295+
mockTime('2024-06-15T00:15:00.000Z');
296+
cache['head:' + url] = {
297+
etag: 'etag-value',
298+
lastModified: 'Fri, 15 Jun 2024 00:00:00 GMT',
299+
httpResponse: { statusCode: 200, body: '' },
300+
timestamp: '2024-06-15T00:00:00.000Z',
301+
};
302+
const cacheProvider = new PackageHttpCacheProvider({
303+
namespace: '_test-namespace',
304+
checkAuthorizationHeader: false,
305+
checkCacheControlHeader: false,
306+
});
307+
httpMock.scope(url).head('').reply(500);
308+
309+
const res = await http.head(url, { cacheProvider });
310+
311+
expect(res.statusCode).toBe(200);
312+
});
313+
314+
it('prevents caching HEAD request when cache-control is private', async () => {
315+
mockTime('2024-06-15T00:00:00.000Z');
316+
317+
const cacheProvider = new PackageHttpCacheProvider({
318+
namespace: '_test-namespace',
319+
checkAuthorizationHeader: false,
320+
checkCacheControlHeader: true,
321+
});
322+
323+
httpMock.scope(url).head('').reply(200, '', {
324+
'cache-control': 'max-age=180, private',
325+
});
326+
327+
const res = await http.head(url, { cacheProvider });
328+
329+
expect(res.statusCode).toBe(200);
330+
expect(packageCache.setWithRawTtl).not.toHaveBeenCalled();
331+
});
332+
333+
it('caches HEAD and GET requests separately', async () => {
334+
const cacheProvider = new PackageHttpCacheProvider({
335+
namespace: '_test-namespace',
336+
checkAuthorizationHeader: false,
337+
checkCacheControlHeader: false,
338+
});
339+
340+
httpMock.scope(url).get('').reply(200, 'get response', {
341+
etag: 'get-etag',
342+
});
343+
httpMock.scope(url).head('').reply(200, '', {
344+
etag: 'head-etag',
345+
});
346+
347+
await http.getText(url, { cacheProvider });
348+
await http.head(url, { cacheProvider });
349+
350+
expect(cache).toEqual({
351+
'http://example.com/foo/bar': {
352+
etag: 'get-etag',
353+
httpResponse: {
354+
statusCode: 200,
355+
headers: expect.any(Object),
356+
body: 'get response',
357+
},
358+
lastModified: undefined,
359+
timestamp: expect.any(String),
360+
},
361+
'head:http://example.com/foo/bar': {
362+
etag: 'head-etag',
363+
httpResponse: {
364+
statusCode: 200,
365+
headers: expect.any(Object),
366+
body: '',
367+
},
368+
lastModified: undefined,
369+
timestamp: expect.any(String),
370+
},
371+
});
372+
});
373+
});
374+
223375
describe('cacheAllowed', () => {
224376
beforeEach(() => {
225377
GlobalConfig.reset();

‎lib/util/http/cache/package-http-cache-provider.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,24 +41,32 @@ export class PackageHttpCacheProvider extends AbstractHttpCacheProvider {
4141
this.checkAuthorizationHeader = checkAuthorizationHeader;
4242
}
4343

44-
async load(url: string): Promise<unknown> {
45-
return await packageCache.get(this.namespace, url);
44+
private cacheKey(method: string, url: string): string {
45+
if (method !== 'get') {
46+
return `${method}:${url}`;
47+
}
48+
return url;
49+
}
50+
51+
async load(method: string, url: string): Promise<unknown> {
52+
return await packageCache.get(this.namespace, this.cacheKey(method, url));
4653
}
4754

48-
async persist(url: string, data: HttpCache): Promise<void> {
55+
async persist(method: string, url: string, data: HttpCache): Promise<void> {
4956
await packageCache.setWithRawTtl(
5057
this.namespace,
51-
url,
58+
this.cacheKey(method, url),
5259
data,
5360
this.hardTtlMinutes,
5461
);
5562
}
5663

5764
override async bypassServer<T>(
65+
method: string,
5866
url: string,
5967
ignoreSoftTtl = false,
6068
): Promise<HttpResponse<T> | null> {
61-
const cached = await this.get(url);
69+
const cached = await this.get(method, url);
6270
if (!cached) {
6371
return null;
6472
}
@@ -110,13 +118,14 @@ export class PackageHttpCacheProvider extends AbstractHttpCacheProvider {
110118
}
111119

112120
override async wrapServerResponse<T>(
121+
method: string,
113122
url: string,
114123
resp: HttpResponse<T>,
115124
): Promise<HttpResponse<T>> {
116125
if (resp.statusCode === 200 && !this.cacheAllowed(resp)) {
117126
return resp;
118127
}
119128

120-
return await super.wrapServerResponse(url, resp);
129+
return await super.wrapServerResponse(method, url, resp);
121130
}
122131
}

‎lib/util/http/cache/repository-http-cache-provider.spec.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Http } from '..';
2-
import { resetCache } from '../../cache/repository';
2+
import { getCache, resetCache } from '../../cache/repository';
33
import { repoCacheProvider } from './repository-http-cache-provider';
44
import * as httpMock from '~test/http-mock';
55

@@ -114,4 +114,48 @@ describe('util/http/cache/repository-http-cache-provider', () => {
114114
authorization: true,
115115
});
116116
});
117+
118+
describe('HEAD requests', () => {
119+
it('caches HEAD requests separately from GET requests', async () => {
120+
const scope = httpMock.scope('https://example.com');
121+
122+
scope
123+
.get('/foo/bar')
124+
.reply(200, { msg: 'GET response' }, { etag: 'get-123' });
125+
scope.head('/foo/bar').reply(200, '', { etag: 'head-123' });
126+
127+
const getRes = await http.getJsonUnchecked('https://example.com/foo/bar');
128+
const headRes = await http.head('https://example.com/foo/bar');
129+
130+
expect(getRes).toMatchObject({
131+
statusCode: 200,
132+
body: { msg: 'GET response' },
133+
});
134+
expect(headRes).toMatchObject({
135+
statusCode: 200,
136+
});
137+
138+
const cache = getCache();
139+
expect(cache.httpCache?.['https://example.com/foo/bar']).toBeDefined();
140+
expect(
141+
cache.httpCacheHead?.['https://example.com/foo/bar'],
142+
).toBeDefined();
143+
});
144+
145+
it('reuses HEAD data with etag', async () => {
146+
const scope = httpMock.scope('https://example.com');
147+
148+
scope.head('/foo/bar').reply(200, '', { etag: 'head-123' });
149+
const res1 = await http.head('https://example.com/foo/bar');
150+
expect(res1).toMatchObject({
151+
statusCode: 200,
152+
});
153+
154+
scope.head('/foo/bar').reply(304);
155+
const res2 = await http.head('https://example.com/foo/bar');
156+
expect(res2).toMatchObject({
157+
statusCode: 200,
158+
});
159+
});
160+
});
117161
});

0 commit comments

Comments
 (0)