markrogoyski / ipv4-subnet-calculator-php

Network calculator for subnet mask and other classless (CIDR) network information.
MIT License
168 stars 42 forks source link

Update SubnetCalculator.php #16

Open xdsta opened 4 years ago

xdsta commented 4 years ago

min-host for /31 and /32 was wrong. max-host for /31 and /32 was wrong.

added private function "addIps".

Attention, min-host and max-host only fixed in decimal-version.

markrogoyski commented 4 years ago

Hi @xdsta,

Thanks for your interest in SubnetCalculator.

Can you provide some information to help me understand what you perceive the issue to be. Please tell me:

Thanks, Mark

xdsta commented 4 years ago

Hey @markrogoyski,

sure - we can use 192.168.0.1/31 and 192.168.0.1/32 for the test as input:

192.168.0.1/32 < this network only has one ip-address, but the min-/max-host-methods return 192.168.0.2 and 192.168.0.1. So max-host is ok, but min-host is wrong because it does not belong to this network

192.168.0.1/31 < this is a special one - it has 2 ip-addresses, but strictly seen no usable hosts. But we can use this for P2P-Connections, so we use the network-address for host 1 and the broadcast for host 2. The min-/max-hostmethods return 192.168.0.1 and 192.168.0.0 - so the output is reversed.

The mistake is to return _$this->ipaddress; in case of an /31 and /32 as it is not sure, that this is the networkaddress. So I changed min-host to return the network-portion instead and max-host to return network-portion + 1 for /31 as a special case.

Greetings xdsta

xdsta commented 4 years ago

If you are not fine with the "+1"-workaround, you can use the broadcast-calculation instead for the max-host-calculation :) This is more comprehensible for all.

} elseif ($this->network_size == 31) { return $this->getBroadcastAddress(); }