Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/sc 123873/check hash #693

Merged
merged 13 commits into from
Nov 30, 2023
23 changes: 13 additions & 10 deletions src/cmd/binary.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ class BinaryCommand {
async inspectBinary(file) {
await this._checkFile(file);
const extractedFiles = await this._extractFiles(file);
const parsedBinaryInfo = await this._parseApplicationBinary(extractedFiles.application);
await this._verifyBundle(parsedBinaryInfo, extractedFiles.assets);
const parsedAppInfo = await this._parseApplicationBinary(extractedFiles.application);
const assets = extractedFiles.assets;
await this._verifyBundle(parsedAppInfo, assets);
}

async _checkFile(file) {
Expand Down Expand Up @@ -85,21 +86,23 @@ class BinaryCommand {
return fileInfo;
}

async _verifyBundle(binaryFileInfo, assets) {
if (binaryFileInfo.assets && assets.length){
async _verifyBundle(appInfo, assets) {
const appAssets = appInfo.assets;
if (appAssets && assets.length > 0) {
console.log('It depends on assets:');
for (const assetInfo of binaryFileInfo.assets) {
const asset = assets.find((asset) => asset.name === assetInfo.name);
for (const appAsset of appAssets) {
const asset = assets.find((bundleAsset) => bundleAsset.name === appAsset.name);
if (asset) {
const valid = isAssetValid(asset.data, assetInfo);
const valid = isAssetValid(asset.data, appAsset);
// isAssetValid();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder to remove commented out code


if (valid) {
console.log(' ' + chalk.bold(assetInfo.name) + ' (hash ' + assetInfo.hash + ')');
console.log(' ' + chalk.bold(appAsset.name) + ' (hash ' + appAsset.hash + ')');
} else {
console.log(chalk.red(' ' + assetInfo.name + ' failed' + ' (hash should be ' + assetInfo.hash + ')'));
console.log(chalk.red(' ' + appAsset.name + ' failed' + ' (hash should be ' + appAsset.hash + ')'));
}
} else {
console.log(chalk.red(' ' + assetInfo.name + ' failed' + ' (hash should be ' + assetInfo.hash + ' but is not in the bundle)'));
console.log(chalk.red(' ' + appAsset.name + ' failed' + ' (hash should be ' + appAsset.hash + ' but is not in the bundle)'));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No functional change here. Felt like the readability of this code could be improved.

}
}
Expand Down
63 changes: 53 additions & 10 deletions src/cmd/flash.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const fs = require('fs-extra');
const os = require('os');
const ParticleApi = require('./api');
const { ModuleInfo } = require('binary-version-reader');
const { ModuleInfo, unwrapAssetModule } = require('binary-version-reader');
const { errors: { usageError } } = require('../app/command-processor');
const usbUtils = require('./usb-util');
const CLICommandBase = require('./base');
Expand All @@ -16,6 +16,7 @@ const { knownAppNames, knownAppsForPlatform } = require('../lib/known-apps');
const { sourcePatterns, binaryPatterns, binaryExtensions } = require('../lib/file-types');
const deviceOsUtils = require('../lib/device-os-version-util');
const semver = require('semver');
const crypto = require('crypto');
const {
createFlashSteps,
filterModulesToFlash,
Expand Down Expand Up @@ -125,40 +126,82 @@ module.exports = class FlashCommand extends CLICommandBase {
const { api, auth } = this._particleApi();
const device = await usbUtils.getOneUsbDevice({ idOrName: deviceIdOrName, api, auth, ui: this.ui });

const platformName = platformForId(device.platformId).name;
const platformId = device.platformId;
const platformName = platformForId(platformId).name;
const currentDeviceOsVersion = device.firmwareVersion;

this.ui.write(`Flashing ${platformName} ${deviceIdOrName || device.id}`);

validateDFUSupport({ device, ui: this.ui });

let { skipDeviceOSFlash, files: filesToFlash } = await this._prepareFilesToFlash({
knownApp,
parsedFiles,
platformId: device.platformId,
platformId,
platformName,
target
});

filesToFlash = await this._processBundle({ filesToFlash });

const fileModules = await parseModulesToFlash({ files: filesToFlash });
await this._validateModulesForPlatform({ modules: fileModules, platformId: device.platformId, platformName });

const eligibleModules = await this._filterAssetsOnDevice(device, fileModules);

await this._validateModulesForPlatform({ modules: eligibleModules, platformId, platformName });

const deviceOsBinaries = await this._getDeviceOsBinaries({
currentDeviceOsVersion: device.firmwareVersion,
currentDeviceOsVersion,
skipDeviceOSFlash,
target,
modules: fileModules,
platformId: device.platformId,
modules: eligibleModules,
platformId,
applicationOnly
});
const deviceOsModules = await parseModulesToFlash({ files: deviceOsBinaries });
let modulesToFlash = [...fileModules, ...deviceOsModules];
modulesToFlash = filterModulesToFlash({ modules: modulesToFlash, platformId: device.platformId });
let modulesToFlash = [...eligibleModules, ...deviceOsModules];
modulesToFlash = filterModulesToFlash({ modules: modulesToFlash, platformId });

const flashSteps = await createFlashSteps({
modules: modulesToFlash,
isInDfuMode: device.isInDfuMode,
platformId
});

const flashSteps = await createFlashSteps({ modules: modulesToFlash, isInDfuMode: device.isInDfuMode , platformId: device.platformId });
await flashFiles({ device, flashSteps, ui: this.ui });
}

async _filterAssetsOnDevice(device, modules) {
if (device.isInDfuMode) {
device = await usbUtils.reopenInNormalMode(device, { reset: true });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use a different approach because this makes flashing less reliable. If the application firmware is crashing and someone manually puts the device in DFU mode to reflash it and the CLI reopens it in normal mode, the device will crash before it can get the asset info. This will also call getAssetInfo even in case the application doesn't have assets.

I think we could create the flash steps as if we're flashing all the assets, but in flashFiles when it comes time to flash an asset, call getAssetInfo (cache the result in case there are other assets) and skip calling device.updateFirmware if the asset already exists.

}
const { available } = await device.getAssetInfo();

const filteredModules = await Promise.all(
modules.map(async (module) => {
if (module.prefixInfo.moduleFunction !== ModuleInfo.FunctionType.ASSET) {
return module; // Keep non-asset modules
}

const hashAssetToBeFlashed = await this._get256Hash(module);
if (available.some((asset) => {
return hashAssetToBeFlashed === asset.hash;
})) {
this.ui.write(`Skipping asset ${path.basename(module.filename)} because it is already on the device`);
return null;
}
return module;
})
);
return filteredModules.filter(Boolean); // Remove null values
}

async _get256Hash(module) {
if (module && module.fileBuffer) {
const assetModule = await unwrapAssetModule(module.fileBuffer);
return crypto.createHash('sha256').update(assetModule).digest('hex');
}
}

async _analyzeFiles(files) {
const apps = knownAppNames();
Expand Down
88 changes: 87 additions & 1 deletion src/cmd/flash.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const { expect, sinon } = require('../../test/setup');
const fs = require('fs-extra'); // Use fs-extra instead of fs
const fs = require('fs-extra');
const nock = require('nock');
const temp = require('temp').track();
const path = require('path');
Expand All @@ -8,6 +8,7 @@ const BundleCommand = require('./bundle');
const { PATH_TMP_DIR } = require('../../test/lib/env');
const deviceOsUtils = require('../lib/device-os-version-util');
const { firmwareTestHelper, createAssetModule, ModuleInfo, HalModuleParser } = require('binary-version-reader');
const usbUtils = require('./usb-util');

describe('FlashCommand', () => {
let flash;
Expand Down Expand Up @@ -418,4 +419,89 @@ describe('FlashCommand', () => {
expect(binaries).to.eql([]);
});
});

describe('_filterAssetsOnDevice and returns eligible modules to be flashed',() => {
let fwModules;
let assetModules;
beforeEach(async () => {
fwModules = await createModules();
assetModules = await createAssetModules();
});

it('returns all firmware modules', async () => {
const device = {
isInDfuMode: false,
getAssetInfo: sinon.stub().resolves({ available: [] }),
};

const eligibleModules = await flash._filterAssetsOnDevice(device, fwModules);

expect(eligibleModules).to.eql(fwModules);
});

it('returns all firmware and asset modules', async () => {
const device = {
isInDfuMode: false,
getAssetInfo: sinon.stub().resolves({ available: [] }),
};
const modules = [...fwModules, ...assetModules];

const eligibleModules = await flash._filterAssetsOnDevice(device, modules);

expect(eligibleModules).to.eql(modules);
});

it('returns all firmware modules and the assets which are not on the device', async () => {
const hash = [
'9e3dd2ea9ff3da70862a52621f7c1dc81c2b184cb886a324a3f430ec11efd3f2',
'8e3dd2ea9ff3da70862a52621f7c1dc81c2b184cb886a324a3f430ec11efd3f2'
];
const device = {
isInDfuMode: false,
getAssetInfo: sinon.stub().resolves({
available: [
{ name: 'asset1.bin', hash: hash[0], size: 16, storageSize: 8 }
]
})
};
const modules = [...fwModules, ...assetModules];
const expectedModules = [...fwModules, assetModules[1]];
sinon.stub(flash, '_get256Hash').callsFake(() => {
const callCnt = flash._get256Hash.callCount;
return Promise.resolve(hash[callCnt - 1]);
});

const eligibleModules = await flash._filterAssetsOnDevice(device, modules);

expect(eligibleModules).to.eql(expectedModules);
});

it('checks if device is in Dfu mode', async () => {
const device = {
isInDfuMode: true,
getAssetInfo: sinon.stub().resolves({ available: [] })
};
const reopen = sinon.stub(usbUtils, 'reopenInNormalMode').resolves(device);

await flash._filterAssetsOnDevice(device, fwModules);

expect(reopen).to.have.been.calledOnce;
});
});

describe('_get256Hash', () => {
it ('returns the hash of the file', async () => {
const assetModules = await createAssetModules();

const hash = await flash._get256Hash(assetModules[0]);

expect(hash).to.equal('8e3dd2ea9ff3da70862a52621f7c1dc81c2b184cb886a324a3f430ec11efd3f2');
});

it ('returns if module is not available', async () => {
const hash = await flash._get256Hash();

expect(hash).to.equal(undefined);
});
});
});