Fix issues with registries (#447)

Author: bwateratmsft

package-lock.json

@@ -3374,7 +3374,7 @@
         "@microsoft/vscode-azext-azureutils": "^4.0.0",
         "@microsoft/vscode-azext-utils": "^4.0.4",
         "@microsoft/vscode-container-client": "^0.5.3",
-        "@microsoft/vscode-docker-registries": "^0.4.1",
+        "@microsoft/vscode-docker-registries": "^0.4.2",
         "@microsoft/vscode-processutils": "^0.2.1",
         "dayjs": "^1.11.7",
         "dockerfile-language-server-nodejs": "^0.15.0",

package.json

@@ -0,0 +1,106 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See LICENSE.md in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert from 'assert';
+import { httpRequest, RequestOptionsLike } from '../../utils/httpRequest';
+
+suite('(unit) utils/httpRequest', () => {
+    let originalFetch: typeof globalThis.fetch;
+    let lastFetchUrl: string | URL | Request;
+    let lastFetchInit: RequestInit | undefined;
+
+    function stubFetch(status = 200, body = '{}', headers?: Record<string, string>): void {
+        globalThis.fetch = async (input: string | URL | Request, init?: RequestInit): Promise<Response> => {
+            lastFetchUrl = input;
+            lastFetchInit = init;
+            return new Response(body, {
+                status,
+                statusText: status === 200 ? 'OK' : 'Error',
+                headers: headers ?? {},
+            });
+        };
+    }
+
+    setup(() => {
+        originalFetch = globalThis.fetch;
+        lastFetchUrl = undefined!;
+        lastFetchInit = undefined;
+    });
+
+    teardown(() => {
+        globalThis.fetch = originalFetch;
+    });
+
+    test('sets duplex to half when body is provided', async () => {
+        stubFetch();
+        const options: RequestOptionsLike = { method: 'POST', body: 'test' };
+        await httpRequest('https://example.com', options);
+
+        assert.strictEqual(lastFetchInit?.duplex, 'half');
+    });
+
+    test('sets duplex to half when body is empty string', async () => {
+        stubFetch();
+        const options: RequestOptionsLike = { method: 'POST', body: '' };
+        await httpRequest('https://example.com', options);
+
+        assert.strictEqual(lastFetchInit?.duplex, 'half');
+    });
+
+    test('does not set duplex when no body is provided', async () => {
+        stubFetch();
+        const options: RequestOptionsLike = { method: 'GET' };
+        await httpRequest('https://example.com', options);
+
+        assert.strictEqual(lastFetchInit?.duplex, undefined);
+    });
+
+    test('does not mutate original options headers via signRequest', async () => {
+        stubFetch();
+        const sharedOptions: RequestOptionsLike = {
+            method: 'HEAD',
+            headers: { 'Accept': 'application/json' },
+        };
+
+        const signRequest = async (request: RequestOptionsLike): Promise<void> => {
+            request.headers!['Authorization'] = 'Bearer token123';
+        };
+
+        await httpRequest('https://example.com', sharedOptions, signRequest);
+
+        // The signed header should have been sent
+        const sentHeaders = lastFetchInit?.headers as Record<string, string>;
+        assert.strictEqual(sentHeaders['Authorization'], 'Bearer token123');
+
+        // But the original options should be unmodified
+        assert.strictEqual(sharedOptions.headers!['Authorization'], undefined, 'Original options.headers should not be mutated');
+        assert.strictEqual(Object.keys(sharedOptions.headers!).length, 1);
+    });
+
+    test('passes signed headers through to fetch', async () => {
+        stubFetch();
+        const options: RequestOptionsLike = {
+            method: 'GET',
+            headers: { 'X-Custom': 'value' },
+        };
+
+        const signRequest = async (request: RequestOptionsLike): Promise<void> => {
+            request.headers!['Authorization'] = 'Bearer mytoken';
+        };
+
+        await httpRequest('https://example.com', options, signRequest);
+
+        const sentHeaders = lastFetchInit?.headers as Record<string, string>;
+        assert.strictEqual(sentHeaders['Authorization'], 'Bearer mytoken');
+        assert.strictEqual(sentHeaders['X-Custom'], 'value');
+    });
+
+    test('passes url directly to fetch', async () => {
+        stubFetch();
+        await httpRequest('https://example.com/v2/test', { method: 'GET' });
+
+        assert.strictEqual(lastFetchUrl, 'https://example.com/v2/test');
+    });
+});

src/test/utils/httpRequest.test.ts

@@ -107,10 +107,8 @@ export class OutdatedImageChecker {
     private async getLatestImageDigest(registry: ImageRegistry, repo: string, tag: string): Promise<string> {
         const manifestResponse = await httpRequest(`${registry.baseUrl}/${repo}/manifests/${tag}`, this.defaultRequestOptions, async (request) => {
             if (registry.signRequest) {
-                return registry.signRequest(request, `repository:library/${repo}:pull`);
+                await registry.signRequest(request, `repository:library/${repo}:pull`);
             }
-
-            return request;
         });
 
         return manifestResponse.headers.get('docker-content-digest') as string;

src/tree/images/imageChecker/OutdatedImageChecker.ts

@@ -6,13 +6,13 @@
 import { ImageNameInfo } from '@microsoft/vscode-container-client';
 import { URL } from 'url';
 import { ociClientId } from '../../../constants';
-import { HttpErrorResponse, IOAuthContext, RequestLike, RequestOptionsLike, bearerAuthHeader, getWwwAuthenticateContext, httpRequest } from '../../../utils/httpRequest';
+import { HttpErrorResponse, IOAuthContext, RequestOptionsLike, bearerAuthHeader, getWwwAuthenticateContext, httpRequest } from '../../../utils/httpRequest';
 import { NormalizedImageNameInfo } from '../NormalizedImageNameInfo';
 
 export interface ImageRegistry {
     isMatch: (imageNameInfo: ImageNameInfo) => boolean;
     baseUrl: string;
-    signRequest?(request: RequestLike, scope: string): Promise<RequestLike>;
+    signRequest?(request: RequestOptionsLike, scope: string): Promise<void>;
 }
 
 let dockerHubAuthContext: IOAuthContext | undefined;
@@ -24,7 +24,7 @@ export const registries: ImageRegistry[] = [
             return !!imageNameInfo.originalName && normalizedImageNameInfo.normalizedRegistry === 'docker.io' && normalizedImageNameInfo.normalizedNamespace === 'library';
         },
         baseUrl: 'https://registry-1.docker.io/v2/library',
-        signRequest: async (request: RequestLike, scope: string): Promise<RequestLike> => {
+        signRequest: async (request: RequestOptionsLike, scope: string): Promise<void> => {
             if (!dockerHubAuthContext) {
                 try {
                     const options: RequestOptionsLike = {
@@ -59,8 +59,8 @@ export const registries: ImageRegistry[] = [
             const tokenResponse = await httpRequest<{ token: string }>(url.toString(), authRequestOptions);
             const token = (await tokenResponse.json()).token;
 
-            request.headers.set('Authorization', bearerAuthHeader(token));
-            return request;
+            request.headers ??= {};
+            request.headers['Authorization'] = bearerAuthHeader(token);
         }
     },
     {

src/tree/images/imageChecker/registries.ts

@@ -16,22 +16,26 @@ export const enum ErrorHandling {
 export async function httpRequest<T>(
     url: string,
     options?: RequestOptionsLike,
-    signRequest?: (request: RequestLike) => Promise<RequestLike>,
+    signRequest?: (request: RequestOptionsLike) => Promise<void>,
     errorHandling: ErrorHandling = ErrorHandling.ThrowOnError
 ): Promise<HttpResponse<T>> {
-    const requestOptions: RequestInit = options;
+    const requestOptions: RequestInit = { ...options, headers: { ...options?.headers } };
     if (options?.form) {
         // URLSearchParams is a silly way to say "it's form data"
         requestOptions.body = new URLSearchParams(options.form);
     }
 
-    let request = new Request(url, requestOptions ?? {});
+    if (requestOptions.body !== undefined && requestOptions.duplex === undefined) {
+        // Node's built-in fetch implementation (via undici) requires `duplex: 'half'`
+        // when sending a request body in this code path, otherwise Node throws an error.
+        requestOptions.duplex = 'half';
+    }
 
     if (signRequest) {
-        request = await signRequest(request) as Request;
+        await signRequest(requestOptions as RequestOptionsLike);
     }
 
-    const response = await fetch(request);
+    const response = await fetch(url, requestOptions);
 
     if (errorHandling === ErrorHandling.ReturnErrorResponse || response.ok) {
         return new HttpResponse(response, url);
@@ -111,12 +115,6 @@ export interface RequestOptionsLike {
     body?: string;
 }
 
-export interface RequestLike {
-    url: string;
-    headers: HeadersLike;
-    method: string; // This is a string because node-fetch's Request defines it as such
-}
-
 export interface HeadersLike {
     get(header: string): string | string[];
     set(header: string, value: string): void;