Improve parsing of IPv6 addresses #1399

pull sipa wants to merge 2 commits into bitcoin:master from sipa:ipparse changing 3 files +119 −43

sipa commented at 8:49 PM on May 30, 2012: member
Implement the following rules:
- Interpret [X]:Y as host=X port=Y, if Y is an integer
- Interpret X:Y as host=X port=Y, if Y is an integer and X contains no colon
- Interpret X:Y as host=X:Y port=default otherwise
in src/netbase.cpp:None in 75c2c0ee3f outdated
```
  35 | @@ -36,6 +36,24 @@ enum Network ParseNetwork(std::string net) {
  36 |      return NET_UNROUTABLE;
  37 |  }
  38 |  
  39 | +void static SplitHostPort(std::string in, int &portOut, std::string &hostOut) {
  40 | +    size_t colon = in.find_last_of(':');
  41 | +    // if a : is found, and it either follows a [...], or no other : is in the string, treat it as port separator
  42 | +    if (colon != in.npos && ((in[0]=='[' && in[colon-1]==']') || colon==0 || in.find_last_of(':',colon-1)==in.npos)) {
```
gavinandresen commented at 10:45 PM on May 30, 2012:

Are you sure the input string is well-formed? What if it is the empty string, or ":" ? Either assertions or sanity checks would be a good idea to avoid indexing out-of-bounds. And maybe unit tests for this routine...
sipa commented at 9:10 AM on May 31, 2012:

It deals with any string:

in[0]=='[': the fact that colon is not npos implies that in is longer than 0 charcters.

in[colon-1]==']': a colon was found and in[0] is not a colon, so colon is not 0.

in.find_last_of(':',colon-1): colon is not zero because of the colon==0 case.

I'll add some comments and unit tests, though.
gavinandresen commented at 12:37 PM on June 1, 2012:

Thanks. I agree the logic as it is now won't crash, but without unit tests I think it is much too easy for somebody to modify it in the future an introduce a bug.

(and I think there might be a problem with the empty string down on line 51, where you check if (in[0] ==...) )
sipa commented at 9:06 PM on June 8, 2012: member

Rebased, refactored the code a bit, and added unit tests. Github fails to recognize it as automatically mergable...

Improve parsing of IPv6 addresses

Implement the following rules:
* Interpret [X]:Y as host=X port=Y, if Y is an integer
* Interpret X:Y as host=X port=Y, if Y is an integer and X contains no colon
* Interpret X:Y as host=X:Y port=default otherwise

1e8aeae15a

Add netbase tests c4971e24f9
sipa commented at 3:45 PM on June 14, 2012: member

Added more tests.
Diapolo commented at 6:35 PM on June 18, 2012: none

@Sipa does this fix ::1 considered as invalid, if used in LookupNumeric()?
gavinandresen referenced this in commit c5532e188e on Jun 18, 2012
gavinandresen merged this on Jun 18, 2012
gavinandresen closed this on Jun 18, 2012
sipa commented at 6:39 PM on June 18, 2012: member

@Diapolo yes; there's even some unit tests that test "::1".
Diapolo commented at 6:42 PM on June 18, 2012: none

Good to see it got merged :).
coblee referenced this in commit a4f4478676 on Jul 17, 2012
lateminer referenced this in commit 8ea289f48a on Jan 22, 2019
lateminer referenced this in commit eb8cb624f9 on May 6, 2020
lateminer referenced this in commit d8663a8410 on May 6, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

sipa

gavinandresen

Diapolo