Skip to content

Commit

Permalink
Fixed issue where filenames with spaces were being unnecessarily esca…
Browse files Browse the repository at this point in the history
…ped when scanning multiple files or a directory in local scan mode. Fixes #126
  • Loading branch information
kylefarris committed Jul 22, 2024
1 parent 6d11a83 commit 8fcaaac
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 11 deletions.
14 changes: 4 additions & 10 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1587,16 +1587,13 @@ class NodeClam {

// Use this method when scanning using local binaries
const localScan = async (allFiles) => {
// Get array of escaped file names
const items = allFiles.map((file) => file.replace(/ /g, '\\ '));

// Build the actual command purely for debugging purposes
const command = `${self.settings[self.scanner].path} ${self._buildClamArgs(items).join(' ')}`;
const command = `${self.settings[self.scanner].path} ${self._buildClamArgs(allFiles).join(' ')}`;
if (self.settings.debugMode)
if (self.settings.debugMode) console.log(`${self.debugLabel}: Configured clam command: ${command}`);

// Execute the clam binary with the proper flags
execFile(self.settings[self.scanner].path, self._buildClamArgs(items), (err, stdout, stderr) => {
execFile(self.settings[self.scanner].path, self._buildClamArgs(allFiles), (err, stdout, stderr) => {
if (self.settings.debugMode) console.log(`${this.debugLabel}: stdout:`, stdout);

// Exit code 1 just means "virus found". This is not an "error".
Expand Down Expand Up @@ -2004,10 +2001,7 @@ class NodeClam {
: resolve({ stderr, path, isInfected: null, goodFiles: [], badFiles: [], viruses: [] });
}

const files = stdout
.trim()
.split(os.EOL)
.map((p) => p.replace(/ /g, '\\ ').trim());
const files = stdout.trim().split(os.EOL);
const { goodFiles, badFiles, viruses, errors } = await this.scanFiles(files, null, null);
return hasCb
? endCb(null, goodFiles, badFiles, viruses)
Expand Down Expand Up @@ -2122,7 +2116,7 @@ class NodeClam {
}

// Get the proper recursive list of files from the path
const files = stdout.split('\n').map((p) => p.replace(/ /g, '\\ '));
const files = stdout.split('\n');

// Send files to remote server in parallel chunks of 10
const chunkSize = 10;
Expand Down
85 changes: 84 additions & 1 deletion tests/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ describe('isInfected', () => {

it('should be okay when scanning a file with consecutive (or any) spaces in it while doing a local scan', async () => {
// Make sure we're forced to scan locally
clamscan = await resetClam({ clamdscan: { host: null, port: null, socket: null, localFallback: true }, debugMode: true });
clamscan = await resetClam({ clamdscan: { host: null, port: null, socket: null, localFallback: true }, debugMode: true, quarantineInfected: false });

// Write virus file with spaces in its name
eicarGen.writeFileSpaced();
Expand Down Expand Up @@ -1115,6 +1115,61 @@ describe('scanFiles', () => {
);
});
});

describe('async/await api', () => {
it('should provide a list of viruses found if the any of the files in the list is infected', async () => {
eicarGen.writeFile();

try {
const { goodFiles, badFiles, errors, viruses } = await clamscan.scanFiles([badScanFile, goodScanFile]);
expect(goodFiles).to.not.be.empty;
expect(goodFiles).to.be.an('array');
expect(goodFiles).to.have.length(1);
expect(badFiles).to.not.be.empty;
expect(badFiles).to.be.an('array');
expect(badFiles).to.have.length(1);
expect(errors).to.be.eql({});
expect(viruses).to.not.be.empty;
expect(viruses).to.be.an('array');
expect(viruses).to.have.length(1);
expect(viruses[0]).to.match(eicarSignatureRgx);
} catch (err) {
throw err;
} finally {
if (fs.existsSync(badScanFile)) fs.unlinkSync(badScanFile);
}
});
});

describe('edge cases', () => {
it('should be fine when one of the filenames has consecutive spaces in it when locally scanning', async () => {
// Make sure we're forced to scan locally
clamscan = await resetClam({ clamdscan: { host: null, port: null, socket: null, localFallback: true }, debugMode: true, quarantineInfected: false });

eicarGen.writeFile();
eicarGen.writeFileSpaced();

try {
const { goodFiles, badFiles, errors, viruses } = await clamscan.scanFiles([badScanFile, goodScanFile, spacedVirusFile]);
expect(goodFiles).to.not.be.empty;
expect(goodFiles).to.be.an('array');
expect(goodFiles).to.have.length(1);
expect(badFiles).to.not.be.empty;
expect(badFiles).to.be.an('array');
expect(badFiles).to.have.length(2);
expect(errors).to.be.eql({});
expect(viruses).to.not.be.empty;
expect(viruses).to.be.an('array');
expect(viruses).to.have.length(1);
expect(viruses[0]).to.match(eicarSignatureRgx);
} catch (err) {
throw err;
} finally {
if (fs.existsSync(badScanFile)) fs.unlinkSync(badScanFile);
if (fs.existsSync(spacedVirusFile)) fs.unlinkSync(spacedVirusFile);
}
});
});
});

describe('scanDir', () => {
Expand Down Expand Up @@ -1274,6 +1329,34 @@ describe('scanDir', () => {
}).timeout(15000);

// TODO: Write tests for file_callback

describe('edge cases', () => {
it('should work when falling back to local scan and there is a file with consecutive spaces in it', async () => {
// Make sure we're forced to scan locally
clamscan = await resetClam({ clamdscan: { host: null, port: null, socket: null, localFallback: true }, debugMode: true, quarantineInfected: false });

eicarGen.writeFile();
eicarGen.writeFileSpaced();

try {
const { goodFiles, badFiles, viruses } = await clamscan.scanDir(badScanDir);

expect(viruses).to.not.be.empty;
expect(viruses).to.be.an('array');
expect(viruses).to.have.length(1);
expect(viruses[0]).to.match(eicarSignatureRgx);
expect(goodFiles).to.be.an('array');
expect(goodFiles).to.have.length(0);
expect(badFiles).to.be.an('array');
expect(badFiles).to.have.length(2);
} catch (err) {
throw err;
} finally {
if (fs.existsSync(badScanFile)) fs.unlinkSync(badScanFile);
if (fs.existsSync(spacedVirusFile)) fs.unlinkSync(spacedVirusFile);
}
});
})
});

describe('scanStream', () => {
Expand Down

0 comments on commit 8fcaaac

Please sign in to comment.