Set accept header to application/octet-stream for downloading package tarballs

src/cache/DiskBasedPackageCache.ts

@@ -104,13 +104,11 @@ export class DiskBasedPackageCache implements PackageCache {
  */
 async function cleanCachedPackage(packageDirectory: string): Promise<void> {
   if (fs.existsSync(path.join(packageDirectory, 'package'))) {
-    (await fs.readdir(packageDirectory))
-      .filter(file => file !== 'package')
-      .forEach(async file => {
-        await fs.rename(
-          path.join(packageDirectory, file),
-          path.join(packageDirectory, 'package', file)
-        );
-      });
+    for (const file of (await fs.readdir(packageDirectory)).filter(file => file !== 'package')) {
+      await fs.rename(
+        path.join(packageDirectory, file),
+        path.join(packageDirectory, 'package', file)
+      );
+    }
   }
 }

src/registry/FHIRRegistryClient.ts

@@ -29,13 +29,14 @@ export class FHIRRegistryClient implements RegistryClient {
     // Construct URL from endpoint, name, and version
     // See: https://confluence.hl7.org/pages/viewpage.action?pageId=97454344#FHIRPackageRegistryUserDocumentation-Download
     const url = `${this.endpoint}/${name}/${version}`;
+    const headers = { Accept: 'application/octet-stream' };
     this.log('info', `Attempting to download ${name}#${version} from ${url}`);
 
     // Right now, this approach is needed for browser environments
     if (this.isBrowserEnvironment) {
       return new Promise((resolve, reject) => {
         https
-          .get(url, res => {
+          .get(url, { headers }, res => {
             if (res.statusCode < 400) {
               resolve(res);
             } else {
@@ -49,7 +50,7 @@ export class FHIRRegistryClient implements RegistryClient {
     }
 
     // Axios approach should be used the vast majority of the time
-    const res = await axiosGet(url, { responseType: 'stream' });
+    const res = await axiosGet(url, { responseType: 'stream', headers });
     if (res?.status === 200 && res?.data) {
       return res.data;
     }

test/cache/DiskBasedPackageCache.test.ts

@@ -45,39 +45,196 @@ describe('DiskBasedPackageCache', () => {
     it('should clean a malformed fhir package tarball when caching it', async () => {
       const tempCacheFolder = temp.mkdirSync('fpl-test');
       const tempCache = new DiskBasedPackageCache(tempCacheFolder, { log: loggerSpy.log });
+      const expectedTargetFolder = path.join(tempCacheFolder, 'fhir.small.wrong#0.1.0');
+      expect(fs.existsSync(expectedTargetFolder)).toBeFalsy();
       // NOTE: This package has example, xml, package, and other folders at root
       const tarballStream = fs.createReadStream(
         path.join(__dirname, 'fixtures', 'tarballs', 'small-wrong-package.tgz')
       );
-      const targetFolder = await tempCache.cachePackageTarball(
-        'fhir.small.wrong',
-        '0.1.0',
-        tarballStream
-      );
-      expect(targetFolder).toEqual(path.join(tempCacheFolder, 'fhir.small.wrong#0.1.0'));
-      // Check that package JSON hasn't moved
-      const packageJSONPath = path.join(targetFolder, 'package', 'package.json');
-      expect(fs.existsSync(packageJSONPath)).toBeTruthy();
-      const packageJSON = fs.readJSONSync(packageJSONPath);
-      expect(packageJSON.name).toEqual('fhir.small.wrong');
-      expect(packageJSON.version).toEqual('0.1.0');
-      // Check that example, other, and xml folders have moved
-      expect(fs.existsSync(path.join(targetFolder, 'example'))).toBeFalsy();
-      expect(fs.existsSync(path.join(targetFolder, 'package', 'example'))).toBeTruthy();
-      expect(fs.existsSync(path.join(targetFolder, 'other'))).toBeFalsy();
-      expect(fs.existsSync(path.join(targetFolder, 'package', 'other'))).toBeTruthy();
-      expect(fs.existsSync(path.join(targetFolder, 'xml'))).toBeFalsy();
-      expect(fs.existsSync(path.join(targetFolder, 'package', 'xml'))).toBeTruthy();
-      // And do one check to make sure their contents moved too
-      const exampleJSONPath = path.join(
-        targetFolder,
-        'package',
-        'example',
-        'Patient-PatientExample.json'
+      const realRename = fs.rename;
+      const realMove = fs.move;
+      const expectedSiblings = new Set(['example', 'other', 'xml']);
+      const completedSiblings = new Set<string>();
+      const startedRenames: Promise<void>[] = [];
+      let extractionRoot: string | undefined;
+      const failures: unknown[] = [];
+      const renameSpy = jest.spyOn(fs, 'rename');
+      const moveSpy = jest.spyOn(fs, 'move');
+      try {
+        renameSpy.mockImplementation(
+          (source: fs.PathLike, destination: fs.PathLike, callback?: fs.NoParamCallback) => {
+            if (
+              callback === undefined &&
+              typeof source === 'string' &&
+              typeof destination === 'string'
+            ) {
+              const root = path.dirname(source);
+              const sibling = path.basename(source);
+              if (
+                expectedSiblings.has(sibling) &&
+                destination === path.join(root, 'package', sibling) &&
+                (extractionRoot === undefined || root === extractionRoot)
+              ) {
+                extractionRoot = root;
+                const rename = new Promise<void>(resolve => setImmediate(resolve)).then(
+                  async () => {
+                    await realRename(source, destination);
+                    completedSiblings.add(sibling);
+                  }
+                );
+                startedRenames.push(rename);
+                return rename;
+              }
+            }
+            if (callback) {
+              return realRename(source, destination, callback);
+            }
+            return realRename(source, destination);
+          }
+        );
+        moveSpy.mockImplementation(
+          (
+            source: string,
+            destination: string,
+            options?: fs.MoveOptions | fs.NoParamCallbackWithUndefined,
+            callback?: fs.NoParamCallbackWithUndefined
+          ) => {
+            if (destination === expectedTargetFolder) {
+              expect(source).toBe(extractionRoot);
+              expect(completedSiblings).toEqual(expectedSiblings);
+            }
+            if (typeof options === 'function') {
+              return realMove(source, destination, options);
+            }
+            if (callback) {
+              return realMove(source, destination, options, callback);
+            }
+            return realMove(source, destination, options);
+          }
+        );
+        try {
+          const targetFolder = await tempCache.cachePackageTarball(
+            'fhir.small.wrong',
+            '0.1.0',
+            tarballStream
+          );
+          expect(targetFolder).toEqual(path.join(tempCacheFolder, 'fhir.small.wrong#0.1.0'));
+          // Check that package JSON hasn't moved
+          const packageJSONPath = path.join(targetFolder, 'package', 'package.json');
+          expect(fs.existsSync(packageJSONPath)).toBeTruthy();
+          const packageJSON = fs.readJSONSync(packageJSONPath);
+          expect(packageJSON.name).toEqual('fhir.small.wrong');
+          expect(packageJSON.version).toEqual('0.1.0');
+          // Check that example, other, and xml folders have moved
+          expect(fs.existsSync(path.join(targetFolder, 'example'))).toBeFalsy();
+          expect(fs.existsSync(path.join(targetFolder, 'package', 'example'))).toBeTruthy();
+          expect(fs.existsSync(path.join(targetFolder, 'other'))).toBeFalsy();
+          expect(fs.existsSync(path.join(targetFolder, 'package', 'other'))).toBeTruthy();
+          expect(fs.existsSync(path.join(targetFolder, 'xml'))).toBeFalsy();
+          expect(fs.existsSync(path.join(targetFolder, 'package', 'xml'))).toBeTruthy();
+          // And do one check to make sure their contents moved too
+          const exampleJSONPath = path.join(
+            targetFolder,
+            'package',
+            'example',
+            'Patient-PatientExample.json'
+          );
+          expect(fs.existsSync(exampleJSONPath)).toBeTruthy();
+          const exampleJSON = fs.readJSONSync(exampleJSONPath);
+          expect(exampleJSON.id).toEqual('PatientExample');
+        } catch (error) {
+          failures.push(error);
+        }
+        const results = await Promise.allSettled(startedRenames);
+        for (const result of results) {
+          if (result.status === 'rejected') {
+            failures.push(result.reason);
+          }
+        }
+        if (failures.length === 1) {
+          throw failures[0];
+        }
+        if (failures.length > 1) {
+          const details = failures
+            .map(failure =>
+              failure instanceof Error ? (failure.stack ?? failure.message) : String(failure)
+            )
+            .join('\n\n');
+          throw Object.assign(new Error(`Multiple malformed-package test failures:\n${details}`), {
+            errors: failures
+          });
+        }
+      } finally {
+        renameSpy.mockRestore();
+        moveSpy.mockRestore();
+      }
+    });
+
+    it('should reject normalization failures before replacing an existing cached package', async () => {
+      const tempCacheFolder = temp.mkdirSync('fpl-test');
+      const tempCache = new DiskBasedPackageCache(tempCacheFolder, { log: loggerSpy.log });
+      const targetFolder = path.join(tempCacheFolder, 'fhir.small.wrong#0.1.0');
+      const packageFolder = path.join(targetFolder, 'package');
+      const packageJSONPath = path.join(packageFolder, 'package.json');
+      const sentinelPath = path.join(packageFolder, 'sentinel.txt');
+      const sentinelContents = 'Keep the existing cached package.';
+      const oldPackageJSON = {
+        name: 'fhir.small.wrong',
+        version: '0.1.0',
+        description: 'Existing cached package'
+      };
+      await fs.ensureDir(packageFolder);
+      await fs.writeFile(sentinelPath, sentinelContents, 'utf-8');
+      await fs.writeJSON(packageJSONPath, oldPackageJSON);
+      const tarballStream = fs.createReadStream(
+        path.join(__dirname, 'fixtures', 'tarballs', 'small-wrong-package.tgz')
       );
-      expect(fs.existsSync(exampleJSONPath)).toBeTruthy();
-      const exampleJSON = fs.readJSONSync(exampleJSONPath);
-      expect(exampleJSON.id).toEqual('PatientExample');
+      const realRename = fs.rename;
+      const normalizationError = new Error('Normalization rename failed');
+      const expectedSiblings = new Set(['example', 'other', 'xml']);
+      let extractionRoot: string | undefined;
+      let normalizationStarts = 0;
+      const renameSpy = jest.spyOn(fs, 'rename');
+      try {
+        renameSpy.mockImplementation(
+          (source: fs.PathLike, destination: fs.PathLike, callback?: fs.NoParamCallback) => {
+            if (
+              callback === undefined &&
+              typeof source === 'string' &&
+              typeof destination === 'string'
+            ) {
+              const root = path.dirname(source);
+              const sibling = path.basename(source);
+              if (
+                expectedSiblings.has(sibling) &&
+                destination === path.join(root, 'package', sibling) &&
+                (extractionRoot === undefined || root === extractionRoot)
+              ) {
+                extractionRoot = root;
+                normalizationStarts += 1;
+                if (normalizationStarts === 1) {
+                  return Promise.reject(normalizationError);
+                }
+              }
+            }
+            if (callback) {
+              return realRename(source, destination, callback);
+            }
+            return realRename(source, destination);
+          }
+        );
+        await expect(
+          tempCache.cachePackageTarball('fhir.small.wrong', '0.1.0', tarballStream)
+        ).rejects.toBe(normalizationError);
+        expect(normalizationStarts).toBe(1);
+        expect(fs.readFileSync(sentinelPath, 'utf-8')).toBe(sentinelContents);
+        expect(fs.readJSONSync(packageJSONPath)).toEqual(oldPackageJSON);
+        expect(loggerSpy.getAllMessages('info')).not.toContain(
+          `Cached fhir.small.wrong#0.1.0 to ${targetFolder}`
+        );
+      } finally {
+        renameSpy.mockRestore();
+      }
     });
 
     it('should cache a fhir package tarball to the cache folder and replace the existing cached package if applicable', async () => {

test/registry/DefaultRegistryClient.test.ts

@@ -1,3 +1,6 @@
+import path from 'path';
+import fs from 'fs-extra';
+import nock from 'nock';
 import type { BasePackageLoaderOptions } from '../../src/loader/BasePackageLoader';
 import { DefaultRegistryClient } from '../../src/registry/DefaultRegistryClient';
 import { FHIRRegistryClient } from '../../src/registry/FHIRRegistryClient';
@@ -69,4 +72,55 @@ describe('DefaultRegistryClient', () => {
       }).not.toThrow();
     });
   });
+
+  describe('#download', () => {
+    beforeEach(() => {
+      jest.replaceProperty(process, 'env', {
+        ...process.env,
+        FPL_REGISTRY: '',
+        HTTPS_PROXY: '',
+        NO_PROXY: '*'
+      });
+      nock.disableNetConnect();
+    });
+
+    afterEach(() => {
+      nock.cleanAll();
+      nock.enableNetConnect();
+      jest.restoreAllMocks();
+    });
+
+    it('should download an archive instead of metadata from the fallback when the primary returns 404', async () => {
+      const archive = fs.readFileSync(
+        path.join(__dirname, '..', 'cache', 'fixtures', 'tarballs', 'small-package.tgz')
+      );
+      const primary = nock('https://packages.fhir.org').get('/fhir.small/0.1.0').reply(404);
+      const fallback = nock('https://packages2.fhir.org')
+        .get('/packages/fhir.small/0.1.0')
+        .reply(function () {
+          if (this.req.headers.accept === 'application/octet-stream') {
+            return [200, archive, { 'Content-Type': 'application/octet-stream' }];
+          }
+          return [
+            200,
+            {
+              name: 'fhir.small',
+              version: '0.1.0',
+              dist: { tarball: 'https://packages2.fhir.org/packages/fhir.small/0.1.0' }
+            },
+            { 'Content-Type': 'application/json' }
+          ];
+        });
+      const client = new DefaultRegistryClient({ log: loggerSpy.log });
+      const result = await client.download('fhir.small', '0.1.0');
+      const chunks: Buffer[] = [];
+      for await (const chunk of result) {
+        chunks.push(chunk);
+      }
+
+      expect(Buffer.concat(chunks).equals(archive)).toBe(true);
+      primary.done();
+      fallback.done();
+    });
+  });
 });

test/registry/FHIRRegistryClient.test.ts

@@ -194,6 +194,13 @@ describe('FHIRRegistryClient', () => {
 
       it('should get the data of the package when 200 response', async () => {
         const result = await client.download('hl7.terminology.r4', '1.2.3-test');
+        expect(axiosSpy).toHaveBeenCalledWith(
+          'https://packages.fhir.org/hl7.terminology.r4/1.2.3-test',
+          expect.objectContaining({
+            responseType: 'stream',
+            headers: { Accept: 'application/octet-stream' }
+          })
+        );
         expect(loggerSpy.getLastMessage('info')).toBe(
           'Attempting to download hl7.terminology.r4#1.2.3-test from https://packages.fhir.org/hl7.terminology.r4/1.2.3-test'
         );
@@ -278,6 +285,9 @@ describe('FHIRRegistryClient', () => {
 
       it('should get the latest version of a package on the packages server', async () => {
         const latest = await client.download('hl7.terminology.r4', 'latest');
+        expect(axiosSpy).toHaveBeenCalledWith('https://packages.fhir.org/hl7.terminology.r4', {
+          responseType: 'json'
+        });
         expect(loggerSpy.getLastMessage('info')).toBe(
           'Attempting to download hl7.terminology.r4#1.2.3-test from https://packages.fhir.org/hl7.terminology.r4/1.2.3-test'
         );
@@ -462,8 +472,10 @@ describe('FHIRRegistryClient', () => {
         scope.done(); // will throw if the nocked URL was never requested
       });
 
-      it('should get the Readable IncomingMessage when successful using https', async () => {
-        const scope = nock('https://packages.fhir.org')
+      it('should request binary content and get the Readable IncomingMessage using https', async () => {
+        const scope = nock('https://packages.fhir.org', {
+          reqheaders: { Accept: 'application/octet-stream' }
+        })
           .get('/hl7.fhir.small/0.1.0')
           .replyWithFile(
             200,