From 816de5ebce1b6efe441dc86dbec4ea30e50ec144 Mon Sep 17 00:00:00 2001
From: Xavier Jouslin de Noray <xavier.jouslindenoray@savoirfairelinux.com>
Date: Fri, 25 Aug 2023 12:05:58 -0400
Subject: [PATCH] Archive: return a Buffer to avoid implicit cast
Change-Id: Ia8ad18b223a106da538658046d925f6f16f3f938
---
src/controllers/plugins.controller.ts | 18 ++++----
src/interfaces/NotFoundException.ts | 1 +
src/services/certificate_manager.ts | 4 +-
src/services/file_manager.service.ts | 14 +++----
src/services/plugins_manager.service.ts | 55 +++++++++++--------------
tests/certificate.manager.test.ts | 2 +-
tests/plugins.controller.test.ts | 4 +-
tests/plugins.manager.test.ts | 8 ++--
8 files changed, 50 insertions(+), 56 deletions(-)
create mode 100644 src/interfaces/NotFoundException.ts
diff --git a/src/controllers/plugins.controller.ts b/src/controllers/plugins.controller.ts
index 1ff7c39..1f0e5a0 100644
--- a/src/controllers/plugins.controller.ts
+++ b/src/controllers/plugins.controller.ts
@@ -20,6 +20,7 @@ import {PluginsManager} from '../services/plugins_manager.service';
import {type Request, type Response, Router} from 'express';
import fs from 'fs';
import {Service} from 'typedi';
+import {NotFoundException} from '../interfaces/NotFoundException';
import {StatusCodes} from 'http-status-codes';
import {LanguageManager} from '../services/language_manager.service';
@@ -290,13 +291,12 @@ export class PluginsController {
req.params.id,
req.query.arch as string
);
- if (icon === '') {
- console.log('sending not found ' + icon);
+ res.status(StatusCodes.OK).send(icon);
+ } catch (e) {
+ if (e instanceof NotFoundException) {
res.status(StatusCodes.NOT_FOUND).send();
return;
}
- res.status(StatusCodes.OK).send(icon);
- } catch (e) {
res.status(StatusCodes.INTERNAL_SERVER_ERROR).send();
}
});
@@ -394,12 +394,14 @@ export class PluginsController {
req.params.id,
req.query.arch as string
);
- res
- .status(background === '' ? StatusCodes.NOT_FOUND : StatusCodes.OK)
- .send(background === '' ? undefined : background);
+ res.status(StatusCodes.OK).send(background);
return;
} catch (e) {
- res.status(StatusCodes.INTERNAL_SERVER_ERROR).send();
+ if (e instanceof NotFoundException) {
+ res.status(StatusCodes.NOT_FOUND).send();
+ return;
+ }
+ res.status(StatusCodes.NOT_FOUND).send();
}
});
}
diff --git a/src/interfaces/NotFoundException.ts b/src/interfaces/NotFoundException.ts
new file mode 100644
index 0000000..25b4d90
--- /dev/null
+++ b/src/interfaces/NotFoundException.ts
@@ -0,0 +1 @@
+export class NotFoundException extends Error {}
diff --git a/src/services/certificate_manager.ts b/src/services/certificate_manager.ts
index c18cb13..5ea63b4 100644
--- a/src/services/certificate_manager.ts
+++ b/src/services/certificate_manager.ts
@@ -34,8 +34,8 @@ export class CertificateManagerService {
): Promise<X509Certificate> {
return await this.fileManager
.readArchive(path, file)
- .then((content: string) => {
- return new X509Certificate(Buffer.from(content));
+ .then((content: Buffer) => {
+ return new X509Certificate(content);
});
}
diff --git a/src/services/file_manager.service.ts b/src/services/file_manager.service.ts
index 925e0bc..1563492 100644
--- a/src/services/file_manager.service.ts
+++ b/src/services/file_manager.service.ts
@@ -61,16 +61,12 @@ export class FileManagerService {
}
}
- async readArchive(path: string, file: string): Promise<string> {
- try {
- // eslint-disable-next-line
+ async readArchive(path: string, file: string): Promise<Buffer> {
+ // eslint-disable-next-line
const zip = new StreamZip.async({ file: path });
- const content = await zip.entryData(file);
- zip.close();
- return await Promise.resolve(content.toString());
- } catch (e) {
- return await Promise.resolve('');
- }
+ const content = await zip.entryData(file);
+ zip.close();
+ return content;
}
async getStat(path: string): Promise<Stats> {
diff --git a/src/services/plugins_manager.service.ts b/src/services/plugins_manager.service.ts
index 96ff1a5..e4e1afd 100644
--- a/src/services/plugins_manager.service.ts
+++ b/src/services/plugins_manager.service.ts
@@ -20,6 +20,7 @@ import {Service} from 'typedi';
import {FileManagerService} from './file_manager.service';
import {type Plugins} from '../interfaces/plugins';
import {CertificateManagerService} from './certificate_manager';
+import {NotFoundException} from '../interfaces/NotFoundException';
import {basename, extname} from 'path';
@Service()
@@ -216,7 +217,7 @@ export class PluginsManager {
background: string;
}> {
const manifest = JSON.parse(
- await this.fileManager.readArchive(path, 'manifest.json')
+ (await this.fileManager.readArchive(path, 'manifest.json')).toString()
);
return {
id: manifest.id === undefined ? manifest.name : manifest.id,
@@ -231,7 +232,7 @@ export class PluginsManager {
async getIcon(
id: string,
arch: string | undefined = undefined
- ): Promise<string> {
+ ): Promise<Buffer> {
if (this.plugins.length === 0) {
await this.setPlugins();
}
@@ -248,7 +249,7 @@ export class PluginsManager {
plugin.icon === undefined ||
platform === undefined
) {
- return '';
+ throw new NotFoundException('Plugin not found');
}
return await this.fileManager.readArchive(
@@ -321,7 +322,7 @@ export class PluginsManager {
[platformPath]
);
// TODO: should refactor this because it's possible that two plugins in different platform have the same id
- if ( plugin === undefined) {
+ if (plugin === undefined) {
continue;
}
plugins.push(plugin);
@@ -330,7 +331,7 @@ export class PluginsManager {
this.plugins = plugins;
}
- async getPluginBackground(id: string, arch: string): Promise<string> {
+ async getPluginBackground(id: string, arch: string): Promise<Buffer> {
if (this.plugins.length === 0) {
await this.setPlugins();
}
@@ -345,30 +346,23 @@ export class PluginsManager {
plugin.background === undefined ||
platform === undefined
) {
- return '';
+ throw new NotFoundException('Plugin not found');
}
- return await this.fileManager
- .readArchive(
- // eslint-disable-next-line
+ return await this.fileManager.readArchive(
+ // eslint-disable-next-line
__dirname +
- '/../..' +
- process.env.DATA_DIRECTORY +
- '/' +
- plugin.id +
- '/' +
- platform +
- '/' +
- plugin.id +
- '.jpl',
- 'data/' + plugin.background
- )
- .then(data => {
- return data;
- })
- .catch(e => {
- return '';
- });
+ '/../..' +
+ process.env.DATA_DIRECTORY +
+ '/' +
+ plugin.id +
+ '/' +
+ platform +
+ '/' +
+ plugin.id +
+ '.jpl',
+ 'data/' + plugin.background
+ );
}
private addPluginArch(platforms: string[], arches: string[]): void {
@@ -411,12 +405,13 @@ export class PluginsManager {
}
return await this.fileManager
.readArchive(pluginPath, 'data/locale/' + languageFile)
- .then((languageFileContent: string) => {
- if (languageFileContent === '') {
+ .then((languageFileContent: Buffer) => {
+ if (languageFileContent.length === 0) {
return text;
}
- const languageFileContentParsed = JSON.parse(languageFileContent);
-
+ const languageFileContentParsed = JSON.parse(
+ languageFileContent.toString()
+ );
return text.replace(
/(\{\{([^}]+)\}\})/g,
(_: string, p1: string) => {
diff --git a/tests/certificate.manager.test.ts b/tests/certificate.manager.test.ts
index 86ff1bb..eda62e0 100644
--- a/tests/certificate.manager.test.ts
+++ b/tests/certificate.manager.test.ts
@@ -56,7 +56,7 @@ describe('Certificate manager service tests', function () {
const expectedCertificate = await promises.readFile(`${__dirname}/TestCertificate.crt`);
const expectedSubject = 'CN=Test';
- fileManager.readArchive.resolves(expectedCertificate.toString());
+ fileManager.readArchive.resolves(expectedCertificate);
const actual = await certificateManagerService['readCertificate'](path, file);
diff --git a/tests/plugins.controller.test.ts b/tests/plugins.controller.test.ts
index 00a637b..1e20ad0 100644
--- a/tests/plugins.controller.test.ts
+++ b/tests/plugins.controller.test.ts
@@ -177,13 +177,13 @@ describe('Routes', function () {
it("should get icon if the given id is valid", (done) => {
const expectedIcon = 'test';
- pluginsManagerServiceStub.getIcon.resolves(expectedIcon);
+ pluginsManagerServiceStub.getIcon.resolves(Buffer.from(expectedIcon));
request(expressApp)
.get("/icons/test?arch=test")
.expect(StatusCodes.OK)
.then((response) => {
- expect(response.text).toEqual(expectedIcon);
+ expect(response.body).toEqual(Buffer.from(expectedIcon));
done();
});
});
diff --git a/tests/plugins.manager.test.ts b/tests/plugins.manager.test.ts
index 7732666..8d2f54f 100644
--- a/tests/plugins.manager.test.ts
+++ b/tests/plugins.manager.test.ts
@@ -244,7 +244,7 @@ describe('Plugins manager service tests', function () {
version: '1.0.0',
};
- fileManager.readArchive.resolves(JSON.stringify(manifest));
+ fileManager.readArchive.resolves(Buffer.from(JSON.stringify(manifest)));
const actual = await pluginsManagerService['readManifest'](pluginPath);
@@ -280,12 +280,12 @@ describe('Plugins manager service tests', function () {
const expected = 'test';
- fileManager.readArchive.resolves(JSON.stringify(manifest));
- fileManager.readArchive.resolves(expected);
+ fileManager.readArchive.resolves(Buffer.from(JSON.stringify(manifest)));
+ fileManager.readArchive.resolves(Buffer.from(expected));
stub(Object.getPrototypeOf(pluginsManagerService), 'getPlatform').returns(['test']);
const actual = await pluginsManagerService['getIcon']("test", "test");
- expect(actual).toEqual(expected);
+ expect(actual).toEqual(Buffer.from(expected));
});
it('should return an array of plugin versions', async () => {
--
GitLab