268893 - IPv4 address validator (JS)

Status: RESOLVED WONTFIX

(Core :: Networking, defect)

Description

[benc]

based on code from isInNet()

function validateIPv4(ipaddr) {
	var test = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/(ipaddr);
	if (test == null){
		return false;
	}
	else if (test[1] > 255 || test[2] > 255 || test[3] > 255 || test[4] > 255) {
		return false;
	}

	else {
		return true;
	}
}

Originally, I thought this function would also take flags for subsets of the
address space (assignable, broadcast, network, mask, and other reserved values).

Because a lot of that might involve bitwise math and working w/ isInNet(), I've
decided to code that in separate functions, perhaps functions that assume the
string passed this function.

if anyone has thoughts on this, let me know.

Comment 1

[benc]

Attachment: HTML-based test harness

Comment 2

[Nigel McFarlane]

Suggested validation improvements:

- Don't allow 00.001.0.000

- Check 32bit-wise that the address is a valid A, B or C class address.

- N.

Comment 3

[benc]

> - Don't allow 00.001.0.000

I'm uncertain of the legality of these addresses (see the "Microsoft-style"
entry in the test harness, named after the Windows TCP/IP control panel), but in
most cases, the backends I've looked at seem to handle it correctly.

Can you provide an example of a system that would not handle this correctly?
 
> - Check 32bit-wise that the address is a valid A, B or C class address.

Can you be more specific what you mean here? A flag tht would exclude non-class
A,B,C ranges? I'm trying to understand the classe-less addressing world we live
in now.

Comment 4

[Nigel McFarlane]

I take your point on MS-style addresses. I thought that was
a GUI thing; I didn't know those strings propagated internally
as-is. It's certainly not a "canonical form" though.

My understanding of this bug is that it's "Add features that
make sysadmin's lives easier", since they're the users of these
functions. On that basis sysadmins would like to be able to sanity
check what IP they get, since they're in too much of a hurry to
ever code anything properly. More bullets to shoot at IP addresses
would thus be nice. Maybe validateIPv4class(IP,A|B|C) or
some such. Lots of organisations still have C class addresses
that represent their own network or parts thereof. They might
want to go one way for their own network, another for the
military's A or B class or whatever. I dunno.

Also is this:

if ( !test || test[1] | test[2] | test[3] | test[4] > 255 ) {
  return false;
}

more efficient? Generally there'll be no short-circuit boolean
evaluation to benefit the longer version. circa 10 ops instead
of circa 15 ops. Trivial nit.

Just ideas, really. Do whatever.

- N.

Comment 5

[benc]

Nigel: 

I think I understand what you mean, you want a function that says: "is this
address on this subnet?" We have that with the isInNet() function already.

I like the nit, I'm not a strong coder, so I'm looking for nits like this when I
posted.