Page MenuHomePhabricator

IPUtils::isValidRange doesn't consider explicit ranges as valid
Closed, ResolvedPublic

Description

IPUtils::isValidRange() only works with 0.0.0.0\0 type ranges, not 0.0.0.0-0.0.0.0 ranges

This is at odds with other code such as IPUtils::parseRange() which does...

While I understand the regex is probably quicker than parseRange...

Can we update it from

	const RE_IP_RANGE = self::RE_IP_ADD . '\/' . self::RE_IP_PREFIX;

to something like (and same for IPv6)

	const RE_IP_RANGE = self::RE_IP_ADD . '\/' . self::RE_IP_PREFIX . '|' . self::RE_IP_ADD . '-' . self::RE_IP_ADD;

Event Timeline

Change 577774 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/libs/IPUtils@master] Allow explicit ranges in IPUtils::isValidRange()

https://gerrit.wikimedia.org/r/577774

I did something similar at https://gerrit.wikimedia.org/r/#/c/mediawiki/libs/IPUtils/+/522989/, but adding another method instead.

It seems odd to me that some functions in the library accept hyphenated ranges, and others don't...

I did something similar at https://gerrit.wikimedia.org/r/#/c/mediawiki/libs/IPUtils/+/522989/, but adding another method instead.

It seems odd to me that some functions in the library accept hyphenated ranges, and others don't...

It is; it's a bad inconsistency. I believe that all methods should accept explicit ranges, as I thought this was the original intention of the author of that code. However, @Krinkle pointed out that this hyphenated notation might have just been an internal thing. See comment "Sep 7 18:54" at https://gerrit.wikimedia.org/r/#/c/mediawiki/libs/IPUtils/+/522989/.

I did something similar at https://gerrit.wikimedia.org/r/#/c/mediawiki/libs/IPUtils/+/522989/, but adding another method instead.

It seems odd to me that some functions in the library accept hyphenated ranges, and others don't...

It is; it's a bad inconsistency. I believe that all methods should accept explicit ranges, as I thought this was the original intention of the author of that code. However, @Krinkle pointed out that this hyphenated notation might have just been an internal thing. See comment "Sep 7 18:54" at https://gerrit.wikimedia.org/r/#/c/mediawiki/libs/IPUtils/+/522989/.

A quick google suggests it's very much not just some internal thing :)

Change 577774 merged by jenkins-bot:
[mediawiki/libs/IPUtils@master] Allow explicit ranges in IPUtils::isValidRange()

https://gerrit.wikimedia.org/r/577774