[wp-trac] [WordPress Trac] #41083: IP with port number triggers warnings in WP_Community_Events

[WordPress Trac]

#41083: IP with port number triggers warnings in WP_Community_Events
----------------------------------------+-----------------------------
 Reporter:  EatonZ                      |       Owner:
     Type:  defect (bug)                |      Status:  new
 Priority:  normal                      |   Milestone:  4.8.1
Component:  Administration              |     Version:  4.8
 Severity:  normal                      |  Resolution:
 Keywords:  needs-patch good-first-bug  |     Focuses:  administration
----------------------------------------+-----------------------------

Comment (by iandunn):

 I worked on this a bit the other day too, [attachment:41083.1.diff] is as
 far as I got. It needs some more work, though, and I'm not entirely
 confident that it's the best approach, either. [attachment:41083.0.diff]
 might be a better approach, but I haven't had time to take a close look.

 [attachment:41083.1.diff] adds a few unit tests, but I think there might
 be a logic error in the loop, since one of the failing tests seems to be
 getting a completely different IP back. I haven't tested it on IIS
 servers, or those without the filter extension installed.

 I used `filter_var` in `get_unsafe_client_ip()`, because it felt too
 fragile to rely on regex, `strpos`, etc, especially if when trying to use
 a single statement for both IPv4 and IPv6 addresses. `filter_var` is the
 most robust way to differentiate between the two.

 The filter extension is disabled on a fraction of servers, though, so the
 function just returns `false` in those cases, which prevents IP
 geolocation. We could instead opt to avoid the anonymization if the
 extension is disabled, but if we're going to anonymize results, then it
 seems better to be consistent about it, especially when the determining
 factors are outside the user's control. Related #40974.

 On the other hand, though, we could just remove the IP anonymization
 entirely, since CDN requests already expose the user's IP to w.org, see
 #40871 and
 https://wordpress.slack.com/archives/C02RQBWTW/p1496167727833251. If this
 turns out to be a bigger headache than it initially appears, then that
 might be the simplest and most maintenance-friendly option.

 We'd probably still need to strip the port off, though, so that we're
 sending valid input to the API.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/41083#comment:4>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform