Hlepers::ipMatch() fixed warning when passed invalid IP address

[hrach]

• bug fix
• no BC break

This fixes a warning which happened on our production servers. Generally, ipMatch() function should be able to handle invalid input without a warning.

The cause was an invalid header
HTTP_X_FORWARDED_FOR: "192.168.5.237,mcd14281.lax,mcd14281.lax, 64.134.222.215, 185.152.66.10, 100.97.10.64"
which was parsed by RequestFactory and validated against known/allowed proxies.

[dg]

Related to #67

[hrach]

Since ipMatch is a validation function, it should handle invalid input. Personally, I don't really mind if it is by @ or if, but I really think, the fix should be here, not in the request factory.

Also, I've implemented the fix for the variable which is expected to be validated, not the validating "pattern".

[dg]

It is not validation function, it is just detecting function. Why do you think it is validation function?

[hrach]

It validates the first argument against the second. Why do you think it is a detection function? What does is detect?

[dg]

It detects (or determines) whether the IP address matches the mask. It expects IP address and mask.

[hrach]

I have repushed the implementation.

Though, it is unpleasant to contribute. Well, we (I) can feel that you probably don't agree with the way of fixing this, but please, write it down. What do think? What are you going to do? E.g "note to myself, #67, will review later" - nothing hard, but enlights the situation a lot.

[dg]

Simply I think that it is correct that function trigger warning on invalid input and I am talking with you to find out why you have a different opinion. It is rather up to you to explain that this is a bug when it's not obvious.

This new solution seems OK to me. Just I do not know if it's a BC break or not, due to dependency on extension filter.

[hrach]

It is rather up to you to explain that this is a bug when it's not obvious.

That's something that didn't cross my mind, I had no idea that this bug report is not obvious.

Just I do not know if it's a BC break or not, due to dependency on extension filter.

filter_var seems to be core function, isn't it?

[hrach]

Thanks!

[dg]

Thanks!