I noticed that this looked a little odd, the boxes for AC and Battery Power Mode are 1 after the other on the same line, then the 3rd box for unknown power mode is on a new line.
Maybe make them all on their own line, like this?
I was testing code and just doing stuff like:
require_once("zeromq.inc");
in Diagnostics->Command Prompt, PHP Execute
That brings an error because underneath that PHP Execute code it has already included auth.inc
I guess zeromq.inc is used quite separately to the rest of the system, and must be OK just having a "require" here. But it seems safer to always use require_once, just in case it gets called in a new way/sequence.
Comments welcome.
This code looked silly the way it was, with the construct:
$var = $var;
unset($var);
Seems it was accidentally changed to this way many years ago by 588a183b0e
IMHO we want to do the conversion to $config['system']['gateway'] here so that later config conversion will then process it as expected.
Note that in a current (2.2) config there is not $config['system']['gateway'] but that is switched over later in upgrade_021_to_022 - which switches back to $config['interfaces']['wan']['gateway'] and then later config conversions do their thing with that.
I guess this will only effect people who upgrade from some really old config (or monowall?)
This routine seems to go looking to see if the passed-in interface is PPP-style. At the end, if it is not PPP-style then it calls pfsense_ngctl_detach.
This foreach loop in its current state will always exit after the first iteration that is not mode "server". But it looks like it should look through all the 'pppoe' entries until it finds the interface or gets to the end.
In theory the code will sometimes miss setting $found = true when it should have. And thus pfsense_ngctl_detach would get called later for a PPP-style interface.
I noticed this while reviewing for code style guide - it is an example where the indenting shows the intention but there are no curlies to implement it.
that match exactly a whole block of private address space.
e.g. if the user has checked "Do not forward private reverse lookups" and also adds adds a domain override that matches a whole block of private address space, such as:
10.in-addr.arpa -> 10.42.1.1
then we want all reverse lookups in the "10" network to be referred to their DNS server at 10.42.1.1 and reverse lookups of other private addresses to immediately return NXDOMAIN.
Without this change, those referrals were not happening, because there was a "-server=10.in-addr.arpa" to nowhere put on the command line and that was effectively overriding the user-specified domain override!
Forum: https://forum.pfsense.org/index.php?topic=89302.0
If the user put "none" in the 'gateway' field for the DHCP settings of an interface, that would set $add_routers to false at line 742. Coming around the loop again for a subsequent interface, and going through the else line 744, nothing would set $add_routers back to true (actually back to the value originally calculated at line 461).
Use a different var to remember the boolean value calculated at line 461 so it can be remembered and used as needed each time through the loop.
I set Gateway "none" on my LAN and left it blank on OPT1 and came up with the same problem as the forum post - OPT1 got no " option routers" statement in dhcpd.conf
After this code fix it looks good.
is_hostname() and is_domain() allow underscore in the names. So it is possible to have underscore in host names, for example in DHCP server static mapped entries I have some things like:
10.42.3.4 client-pc-01_LAN
10.42.3.5 client-pc-01_WIFI
These reverse-resolve fine - 10.42.3.4 becomes client-pc-01_LAN
But the preg_match here misses such names that have an underscore in them.
I noticed this when looking into forum post: https://forum.pfsense.org/index.php?topic=88956.0
The existing implementation of Net_IPv6::compress produces an empty
string when compressing the all-zeros ("::") address; fix this by
checking for empty return values and replacing them with "::".
The existing code only includes a v6 name server IP in the
automatically generated dhcpdv6 configuration for tracking interfaces if
there are additional prefixes that can be delegated on to the next
router (i.e., if PD length > 2). The correct behavior is for this option
to be included regardless of the PD length, as implemented in this
change.
When computing the start IP for the 'available range' field,
services_dhcpv6.php attempts to increment a colon-formatted v6 address.
Since this always fails, so the value that's printed is not actually
incremented, so remove the bogus addition.
Note that this is actually matches the behavior of the range checks that
services_dhcpv6.php performs: The actual check uses the range
[gen_subnetv6()..gen_subnetv6_max()], which does include the first
(= all-zeroes host part) v6 address in the prefix.
Currently if the user is clicking around while they are setting up an OpenVPN server, they can do stuff like this:
a) Select Server Mode - Remote Access (SSL/TLS + User Auth)
b) Select something in Backend for authentication
c) Change their mind and select Server Mode - Peer to Peer (SSL/TLS)
d) Enter the other settings and Save
Now the OpenVPN server config has an 'authmode' key in it.
Probably does no harm, I suspect it will simply not be used when building the server.conf for Peer to Peer, but it looks a bit odd when analysing/diagnosing a config for problems.
Other fields that are mode-specific have tests to only save the values at the end if the appropriate mode is the one finally selected.
While I am here, I also constantly forget to click on "Local Database" authmode when setting up a new server. It gives the validation error message, then I click on "Local Database" again and save. Seems unnecessary - when defining a new OpenVPN server why not default this to have the first entry in the list be the one selected? So I did that. What do you think? 1 place less for many users to need to click.
IMHO it can be confusing on the DHCP edit page for static mapped entries to know which interface the entry is being edited/added for. Specially if the user comes from the Status DHCP Leases page, the lease they are editing/adding could be for 1 of many LAN-style interfaces.
This also made it easier for me to see which interface entry I was editing when I was testing https://github.com/pfsense/pfsense/pull/1504
The edit button for static entries always has an index id=0 and thus pressing the edit button goes to (mostly) edit the wrong entry.
It was easier and made the code cleaner to get rid of the looping through the staticmap array every time trying to matach IP, MAC address... That data ('if' and 'staticmap_array_index') is easily gathered further up, the first time the staticmap array for each DHCP interface is traversed. I think this should also be more efficient when there are many static mapping entries.
Forum: https://forum.pfsense.org/index.php?topic=89072.msg493241#msg493241
