Skip to content

Commit

Permalink
system: unset gateway here too
Browse files Browse the repository at this point in the history 
PR: https://forum.opnsense.org/index.php?topic=11540.0
  • Loading branch information
@fichtner
fichtner committed May 1, 2019
1 parent a4958ed commit 6adafe8
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/opnsense/mvc/app/library/OPNsense/Routing/Gateways.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,8 @@ public function getGateways()
$this->cached_gateways[$gwkey] = $thisconf;
} elseif (empty($thisconf['dynamic'])) {
$gwkey = $this->newKey($thisconf['priority'], !empty($thisconf['defaultgw']));
// gateway should only contain a valid address, make sure its empty
unset($thisconf['gateway']);
$this->cached_gateways[$gwkey] = $thisconf;
}
}
Expand Down

4 comments on commit 6adafe8

@AdSchellevis
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fichtner this doesn't look right, if there was a "dynamic" in the config, we should probably remove the value above the last line in

core/src/opnsense/mvc/app/library/OPNsense/Routing/Gateways.php

Lines 171 to 183 in 6adafe8

if (Util::isIpAddress($gateway->gateway)) {
if (empty($gw_arr['monitor_disable']) && empty($gw_arr['monitor'])) {
$gw_arr['monitor'] = $gw_arr['gateway'];
}
$gwkey = $this->newKey($gw_arr['priority'], !empty($gw_arr['defaultgw']));
$this->cached_gateways[$gwkey] = $gw_arr;
} else {
// dynamic gateways might have settings, temporary store
if (empty($dynamic_gw[(string)$gateway->interface])) {
$dynamic_gw[(string)$gateway->interface] = array();
}
$dynamic_gw[(string)$gateway->interface][] = $gw_arr;
}

@fichtner
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any progress here would be fine. Right now the code is not in sync with what it did before (and before is "strange" to say the least).

Now though I could not see gateways stored in the config.xml which had no router file... which at least makes deleting them impossible. So I added the elseif but forgot to unset the stale gateway value.

Then there's the dynamic thing, which used to be a placeholder for dynamic gateway that is even injected into the config.xml I think, but I can't double-check tonight since I'm away from home.

Let me know what you need to debug this further.

@AdSchellevis
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fichtner I'm not sure how to replicate the issue, but I do think I know what the issue is, let me prepare a fix by moving the line and let me know if it doesn't fix the issue you've seen before.

@AdSchellevis
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fichtner my bad, now I seem to have misread the block. I'll revert my "fix"

Please sign in to comment.