+++ This bug was initially created as a clone of Bug #80855 comment 74 +++ neil@parkwaycc.co.uk 2012-07-08 17:48:58 CEST *Rather like IPv6 not requiring all components (::0:: assumed), IPv4 does not require all components either. In this case, the last component can be a larger integer to make up the missing bytes. Decimal, octal and hex are all legal. Some examples in hex: 0xC0.0xA8.0x90.0x78 can be written 0xC0.0xA8.0x9078 or 0xC0.0xA89078 or 0xC0A89078 and they are all the same address.

From bug 810763: neil@parkwaycc.co.uk 2013-04-07 19:25:11 CEST Comment on attachment 734291 [details] [diff] [review] [diff] [review] test for 80855 Seems reasonable, but do you want to add things like 127.1 and 10.100.1000 as valid extended IP addresses?

Let's not forget to add those new formats to the test in mailnews/base/test/unit/test_hostnameUtils.js .

Apply on top of patch in bug 810763.

Comment on attachment 734766 [details] [diff] [review] patch >+ if ((componentCount != 4 && !aAllowExtendedIPFormats) || componentCount > 4) >+ return null; Not 100% sure about this. Perhaps if ((componentCount < 4 && !aAllowExtendedIPFormats) || componentCount > 4) or if (aAllowExtendedIPFormats ? componentCount > 4 : componentCount != 4) >+ let component = 0; Nit: = 0 is unnecessary. >+ // Make sure the component in not larger then the expected maximum. >+ if (component > Math.pow(256, aWidth) - 1) if ((component >>> ((aWidth - 1) * 8)) >= 256) perhaps?

(mostly to avoid Math.pow)

OK, if I am allowed to put a comment there what that crazy thing does ;)

(In reply to neil@parkwaycc.co.uk from comment #4) > >+ // Make sure the component in not larger then the expected maximum. > >+ if (component > Math.pow(256, aWidth) - 1) > if ((component >>> ((aWidth - 1) * 8)) >= 256) perhaps? The page at http://stackoverflow.com/questions/2373791/bitshift-in-javascript mentions shift arguments are cut down to 32bits. May this expression lie to us if component is above max_uint32?

(In reply to aceman from comment #7) > (In reply to neil@parkwaycc.co.uk from comment #4) > > >+ // Make sure the component in not larger then the expected maximum. > > >+ if (component > Math.pow(256, aWidth) - 1) > > if ((component >>> ((aWidth - 1) * 8)) >= 256) perhaps? > shift arguments are cut down to 32bits. May this expression lie to us if > component is above max_uint32? Ah, that's a point. In that case, I guess if (component >= Math.pow(256, aWidth)) (to avoid the unnecessary -1)

But we can always have a precomputed table for the lookup: const kPowersOf256 = [ 256, 65535, 16777216, 4294967296] if (component >= kPowersOf256[aWidth]) ...

Neil, ping :)

Comment on attachment 736383 [details] [diff] [review] patch v2 >+ if (!((aAllowExtendedIPFormats && componentCount <= 4) || componentCount == 4)) >+ return null; I found this confusing. A possible alternative approach: if (componentCount > 4) return null; if (componentCount < 4 && !aAllowExtendedIPFormats) return null; (Or you could use || if you don't think that makes it confusing again). >+ const kPowersOf256 = [ 1, 256, 65535, 16777216, 4294967296 ]; 65536. r=me with that fixed.

Thanks, done.