net/upnp: Complete service improvements 2/2#5256
net/upnp: Complete service improvements 2/2#5256Self-Hosting-Group wants to merge 3 commits intoopnsense:masterfrom
Conversation
| } | ||
|
|
||
| if (!empty($upnp_config['allow_third_party_mapping'])) { | ||
| if (($upnp_config['allow_third_party_mapping'] ?? '') == '1' || ($upnp_config['allow_third_party_mapping'] ?? '') == 'upnp-igd') { |
There was a problem hiding this comment.
feels like this overcomplicates things and imposes "upnp-igd" on the meaning when in reality we're flipping pcp with this and the secure mode simply is handled as a separate toggle?
There was a problem hiding this comment.
It may look overcomplicated to you, but that's only because the daemon initially had the meaningless option name secure_mode for UPnP IGD, PCP support was added later. secure_mode only applies to UPnP IGD, whereas pcp_allow_thirdparty (reversed) only applies to PCP. The same option is also available in OpenWrt, and I cannot imagine why anyone would make two options out of it.
There was a problem hiding this comment.
Fair, still don't like the handling of this with four times ?? '' and two times checking the same flag. something that an in_array() can do much more concisely.
Self-Hosting-Group
force-pushed
the
not-applied
branch
4 times, most recently
from
7216aa4 to
b0852db
Compare
Self-Hosting-Group
changed the title
| $fw->registerAnchor('miniupnpd', 'fw'); // required for IPv6 | ||
| $fw->registerAnchor('miniupnpd', 'nat', 0, 'head'); | ||
| $fw->registerAnchor('miniupnpd', 'binat'); | ||
| // $fw->registerAnchor('miniupnpd', 'binat'); |
There was a problem hiding this comment.
If it's not used just remove it
| // $fw->registerAnchor('miniupnpd', 'binat'); |
| default: | ||
| break; | ||
| case 'debug': | ||
| $cmd_frmt[] = '-vv'; |
There was a problem hiding this comment.
need evidence -v -v works differently than -vv
|
|
||
| 1.9 | ||
|
|
||
| * INCOMPLETE: Log file, help, wording, logging patch |
There was a problem hiding this comment.
the release for 26.1.3 is Wednesday so the PR should be ready by tomorrow.
2 participants
No description provided.