kylefarris/clamscan

Invalid information provided to connect to clamav service

niftylettuce opened this issue · 6 comments

@kylefarris it seems like #57 and #58 does not resolve the issue I had described. Below is the stack trace:

Invalid information provided to connect to clamav service. A unix socket or port (+ optional host) is required!
Error: Invalid information provided to connect to clamav service. A unix socket or port (+ optional host) is required!
   at /var/www/production/source/node_modules/clamscan/index.js:1649:29
   at new Promise (<anonymous>)
   at NodeClam.scan_stream (/var/www/production/source/node_modules/clamscan/index.js:1636:16)
   at /var/www/production/source/node_modules/spamscanner/index.js:400:32
   at Array.map (<anonymous>)
   at SpamScanner.getVirusResults (/var/www/production/source/node_modules/spamscanner/index.js:392:26)
   at async Promise.all (index 4)
   at async SpamScanner.scan (/var/www/production/source/node_modules/spamscanner/index.js:1144:9)
   at async MessageSplitter.<anonymous> (/var/www/production/source/index.js:1087:18)

You can reference the source code at https://github.com/spamscanner/spamscanner to see how I implemented it, and let me know maybe if my usage is wrong? This to me seems like a bug with clamscan, since the config passed clearly has the socket defined, and the error message indicates it is instead not defined.

This is most likely from stuff happening in parallel to me it seems at least.

Actually, I think there might be an issue with your code...

In the following section, you're doing a Promise.all() but you don't need to since the map you're running doesn't return any unresolved promises. In fact, it doesn't return anything (which is invalid).

const clamscan = await this.clamscan.init(this.config.clamscan);
await Promise.all(
  mail.attachments.map(async (attachment, i) => {
    try {
      const stream = isStream(attachment.content)
        ? attachment.content
        : intoStream(attachment.content);
      const {
        is_infected: isInfected,
        viruses
      } = await clamscan.scan_stream(stream);
      const name = isSANB(attachment.filename)
        ? `"${attachment.filename}"`
        : `#${i + 1}`;
      if (isInfected)
        messages.push(
          `Attachment ${name} was infected with "${viruses}".`
        );
    } catch (err) {
      this.config.logger.error(err);
    }
  })
);

Something along these lines might be better...

const clamscan = await this.clamscan.init(this.config.clamscan);
try {
  const results = await Promise.all(
    mail.attachments.map((attachment, i) => {
      const stream = isStream(attachment.content)
        ? attachment.content
        : intoStream(attachment.content);
      return clamscan.scan_stream(stream);
    })
  );

  results.forEach((result, i) => {
    const attachment = mail.attachments[i];
    const name = isSANB(attachment.filename)
      ? `"${attachment.filename}"`
      : `#${i + 1}`;
    if (result.isInfected)
      messages.push(
        `Attachment ${name} was infected with "${result.viruses}".`
      );
  });
} catch (err) {
    this.config.logger.error(err);
}

Let me know if that helps at all.

Did this help at all? I'm inclined to close this ticket in a few days under the assumption that you've figured out what's going on.

No, that did not help. You can use await Promise.all even if you don't return anything, that's totally valid.

You’re misunderstanding what I’m saying. It’s not the Promise.all that’s the problem, it’s you’re map. Nothing is actually going into your Promise.all method because the map isn’t retuning anything.

The way you currently have it, you might as well just run a forEach loop and skip the Promise.all altogether.

FWIW, I just wrote some tests to verify that the specific scenario you have is working and the tests pass. You can see the new tests here:

clamscan/tests/index.js

Lines 1262 to 1302 in 7ea77ae

it('should not fail when run within a Promise.all()', async () => {
clamscan = await reset_clam();
const [result1, result2] = await Promise.all([
clamscan.scan_stream(get_good_stream()),
clamscan.scan_stream(get_bad_stream()),
]);
expect(result1.is_infected).to.be.a('boolean');
expect(result1.is_infected).to.eql(false);
expect(result1.viruses).to.be.an('array');
expect(result1.viruses).to.have.length(0);
expect(result2.is_infected).to.be.a('boolean');
expect(result2.is_infected).to.eql(true);
expect(result2.viruses).to.be.an('array');
expect(result2.viruses).to.have.length(1);
});
it('should not fail when run within a weird Promise.all() (issue #59)', async () => {
clamscan = await reset_clam();
const items = [get_good_stream(), get_bad_stream()];
await Promise.all(
items.map(async (v,i) => {
const {is_infected, viruses} = await clamscan.scan_stream(v);
if (i === 0) {
expect(is_infected).to.be.a('boolean');
expect(is_infected).to.eql(false);
expect(viruses).to.be.an('array');
expect(viruses).to.have.length(0);
} else {
expect(is_infected).to.be.a('boolean');
expect(is_infected).to.eql(true);
expect(viruses).to.be.an('array');
expect(viruses).to.have.length(1);
}
})
);
});

There may be something else going on in your code but I'm not sure how to help beyond this. As far as I can test, the clamscan module is working fine. If you can provide a simplified repeatable test script showing that this module isn't working as expected, I'd be more than happy to address it (pull requests are also welcome as always).

Thanks for following up here, will close, and follow up if I see it again.