# Bug 47435[Was]: Re:  What gruntwork needs to be done

- Thread: https://externals.io/message/47827
- Original archive: https://news-web.php.net/php.internals/47827
- Started by Justin Dearing on 2010-04-07T19:30:10+00:00
- Messages: 5
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #47827 — Justin Dearing — 2010-04-07T19:30:10+00:00

URL: https://externals.io/message/47827#47827

Ramus,



>
> Here is a straightforward (but not easy) one:
>
> http://bugs.php.net/bug.php?id=47435
>
> I actually found it easy, slightly tedious, and not straightforward. I
guess its perspective.

I am CCing my former colleague Mike G who added is $.02 to the ticket at my
request.

I have submitted a patch that includes tests after Valli's suggestion. I
have to add the IPv4 mapping code, and will do that tonight. If you could
provide feedback on the rest I would appreciate it.

Below are two proposals that I think belong in separate tickets. I will open
these two tickets unless they are summarily rejected by Ramus.

Valli's notes implied that `FILTER_VALIDATE_IP` should match CIDR netmask
notations. eg 0.0.0.0/0 and ::1/128 should match. They do not. I propose a
new flag called FILTER_FLAG_INCLUDE_CIDR_NETMASK that does match.

I second Mike G's proposed additional "FILTER_FLAG_NO_SPECIAL_RANGE" that
captures the union of the other sets as a convenient shortcut.

Justin

---

## #47828 — Rasmus Lerdorf — 2010-04-07T19:39:50+00:00

URL: https://externals.io/message/47827#47828 · In reply to: #47827

> Ramus,
> 
> On Tue, Apr 6, 2010 at 1:34 PM, Rasmus Lerdorf <rasmus@lerdorf.com
> <mailto:rasmus@lerdorf.com>> wrote:
> 
>     Here is a straightforward (but not easy) one:
> 
>     http://bugs.php.net/bug.php?id=47435
> 
> I actually found it easy, slightly tedious, and not straightforward. I
> guess its perspective.
> 
> I am CCing my former colleague Mike G who added is $.02 to the ticket at
> my request.
> 
> I have submitted a patch that includes tests after Valli's suggestion. I
> have to add the IPv4 mapping code, and will do that tonight. If you
> could provide feedback on the rest I would appreciate it.
> 
> Below are two proposals that I think belong in separate tickets. I will
> open these two tickets unless they are summarily rejected by Ramus.
> 
> Valli's notes implied that `FILTER_VALIDATE_IP` should match CIDR netmask
> notations. eg 0.0.0.0/0 <http://0.0.0.0/0> and ::1/128 should match.
> They do not. I propose a new flag called
> FILTER_FLAG_INCLUDE_CIDR_NETMASK that does match. 
> 
> I second Mike G's proposed additional "FILTER_FLAG_NO_SPECIAL_RANGE"
> that captures the union of the other sets as a convenient shortcut.

I'm ok with these.  Like you say, this stuff is a bit tedious to sort
through, and anything we can do to make it easier on users is a good
thing.  I'm trying to get the filter extension beefed up in the next
version of PHP so people will gravitate towards it as an easy to use
built-in mechanism as opposed to rolling their own over and over.

-Rasmus

---

## #47830 — Justin Dearing — 2010-04-07T20:13:02+00:00

URL: https://externals.io/message/47827#47830 · In reply to: #47828

>
> I'm ok with these.  Like you say, this stuff is a bit tedious to sort
> through, and anything we can do to make it easier on users is a good
> thing.  I'm trying to get the filter extension beefed up in the next
> version of PHP so people will gravitate towards it as an easy to use
> built-in mechanism as opposed to rolling their own over and over.
>

I'm all about reusing code.

Before I open the ticket, can someone propose a better constant name than
FILTER_FLAG_INCLUDE_CIDR_NETMASK.

Justin

---

## #47831 — Michael Graziano — 2010-04-07T21:18:20+00:00

URL: https://externals.io/message/47827#47831 · In reply to: #47827

> Ramus,
>
> 
>
> > […]
> guess its perspective.
>
> I am CCing my former colleague Mike G who added is $.02 to the ticket at
> my
> request.

> Valli's notes implied that `FILTER_VALIDATE_IP` should match CIDR netmask
> notations. eg 0.0.0.0/0 and ::1/128 should match. They do not. I propose a
> new flag called FILTER_FLAG_INCLUDE_CIDR_NETMASK that does match.

I have mixed feelings on the CIDR matching aspect -- using CIDR notation
It's possible to define an "insane" network (like 8000::/1 or 128.0.0.0/1
or 192.0.0.0/8) that encompasses a bunch of "special" nets, but is not
itself "special" per the RFCs. It's a stupid and pathological edge case
which should never happen, but it creates all sorts of funky interaction
with the other filter flags that you would need to account for.
Rejecting the CIDR mask and requiring an unqualified (and therefore
implied v4/32 or v6/128) address is a neat (and common) way of
sidestepping that problem.

Having said that, in the sane, non-pathological case subnet matching is
really nice to have. :)

-MG

---

## #47832 — Ferenc Kovacs — 2010-04-08T04:45:46+00:00

URL: https://externals.io/message/47827#47832 · In reply to: #47831

I want both!!!
:)

Tyrael



> > […]
> wrote:
> > […]
>
> > […]
> a
> > […]
>
> I have mixed feelings on the CIDR matching aspect -- using CIDR notation
> It's possible to define an "insane" network (like 8000::/1 or 128.0.0.0/1
> or 192.0.0.0/8) that encompasses a bunch of "special" nets, but is not
> itself "special" per the RFCs. It's a stupid and pathological edge case
> which should never happen, but it creates all sorts of funky interaction
> with the other filter flags that you would need to account for.
> Rejecting the CIDR mask and requiring an unqualified (and therefore
> implied v4/32 or v6/128) address is a neat (and common) way of
> sidestepping that problem.
>
> Having said that, in the sane, non-pathological case subnet matching is
> really nice to have. :)
>
> -MG
