----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46879/#review131683 -----------------------------------------------------------
proton-c/src/reactor/connection.c (line 338) <https://reviews.apache.org/r/46879/#comment195722> This is not a safe test. A DNS name can have digits in it. e.g. 0foo.example.com would be incorrectly dropped. IMO proton shouldn't be interpreting the hostname at all, just copying it. A normal user client will set a valid hostname in the URL, a dodgy developer setup should manually override the hostname if it is different from the URL (which it almost never will be). Proton should do nothing but copy the URL host name and let the user override it if they want to, it should not try to "guess" how to mangle or delete "special" hostnames. If we absolutely must do it we need a full-on RFC-compliant IPV4 and IPV6 parse to make sure we only drop things that are raw IP addresses. proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java (line 468) <https://reviews.apache.org/r/46879/#comment195723> Same as earlier comment re. testing for IP addresses. There's a problem with how we choose to ignore addresses. - Alan Conway On May 1, 2016, 11:54 p.m., Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46879/ > ----------------------------------------------------------- > > (Updated May 1, 2016, 11:54 p.m.) > > > Review request for qpid, Chug Rolke, Justin Ross, and Robbie Gemmell. > > > Bugs: PROTON-1181 > https://issues.apache.org/jira/browse/PROTON-1181 > > > Repository: qpid-proton-git > > > Description > ------- > > See linked JIRA > > > Diffs > ----- > > proton-c/src/engine/engine.c c620101 > proton-c/src/reactor/connection.c 336d1f1 > proton-c/src/tests/reactor.c 9564569 > proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java > d13cfbe > tests/python/proton_tests/reactor.py 6ee107d > > Diff: https://reviews.apache.org/r/46879/diff/ > > > Testing > ------- > > New unit tests added. > > > Thanks, > > Kenneth Giusti > >
