Port range is not always respected

[sholladay]

Allowing users to set even one part of the range for findAPortNotInUse can lead to weird results, unless we manually look at their value. Developers should be able to rely upon portscanner to handle the math for us to ensure the range used is within the range given, regardless of which number happens to be greater.

Assuming that ports 4443 and 4444 are currently in use / "open", this code...

portscanner.findAPortNotInUse(4443, 4440, '127.0.0.1', function(error, port) {
    console.log('Available port: ' + port);
});

... will log "Available port: 4445"

[EndangeredMassa]

I'll take a look!

[sholladay]

FYI, I have posted a bounty for this issue on Bountysource.
https://www.bountysource.com/issues/4358248-port-range-is-not-always-respected

[kbailey4444]

So in the example should just ports 4443 and 4440 be checked, or should 4440-4443 ?

[sholladay]

The behavior I expect is for portscanner to figure out which number is greater and use it as the max port, while the other gets used as the min port. So in my example, ports 4440-4443 would be checked, as if I called the API that way to begin with.

[kbailey4444]

Solution here:
#29

[sholladay]

Thanks @kbailey4444, that looks like what I wanted. If @EndangeredMassa accepts your patch, the bounty is yours.

[laggingreflex]

I'm sorry about the delay on this. I've just been added as a new collaborator and would like to get all pending issues sorted out.

My thoughts about the existing solutions to this issue, that aim to switch the order, is that it could lead to ambiguity. Someone might expect 4443-4440 to also be checked in that order (highest to lowest), just switching the numbers might make for a bad assumption from portscanner's side.

How about throwing an error instead? This could alert the programmer to use portscanner more correctly.

[sholladay]

How about throwing an error instead?

I could live with that.

I had been thinking about String#substring() as an example of argument swapping.

If indexStart is greater than indexEnd, then the effect of substring() is as if the two arguments were swapped; for example, str.substring(1, 0) == str.substring(0, 1).

But it's true that the order of the in-between numbers might matter a bit more to portscanner users if they really wanted to walk them in a special way.

My feelings on this aren't very strong. But another point in favor of swapping vs throwing is that the former can be a semver patch or minor, while the later is typically done as a semver major as it can halt a program.

[laggingreflex]

You're right, it does make more practical sense to just swap, even if at least for now. Maybe in the future we could actually support scanning in the order ranges are supplied. For now I've added swap functionality.

Released a new minor version 1.1.0, please check it out and let me know.

Edit: Apologies to other contributors, I had to add this myself due to other changes and conflicting merges.

[laggingreflex]

It turns out v1.1.0 broke portscanner in some cases and the changes that broke it were actually the changes required to fix this exact issue. For that reason I've published the new changes as v2.0.0. I'm sorry this fix couldn't make it a minor version. I've addressed the issue here.