Use null instead of default, to avoid stomping on netmask #23382
Conversation
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi @marksteward
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
home-assistant
bot
added
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
| @@ -656,8 +656,10 @@ export class HassioNetwork extends LitElement { | |||
| if (id === "address") { | |||
| const index = (ev.target as any).index as number; | |||
| const { mask } = parseAddress(value); | |||
| this._interface[version]!.address![index] = formatAddress(value, mask); | |||
| this.requestUpdate("_interface"); | |||
| if (mask) { | |||
There was a problem hiding this comment.
This means a change in IP address is never picked up as long as there is no mask? And will never be saved?
Also means that if there is a re-render before the mask is updated, the changes of the user are undone.
There was a problem hiding this comment.
Ahh yep, just realised this after playing some more. It looks like adding an empty IP address adds with a /24 or /64 netmask, so there's always one behind the scenes, which we can use as the default instead.
|
(I've run |
To get CI green, you may need to run prettier as well
|
|
Thanks, that looks better! |
Proposed change
Refines #23348 to return
nullinstead of a default netmask. This then allows the code to avoid stomping on a preexisting netmask that isn't /24 for IPv4 or /64 for IPv6.To be clear, the issue was never that the default was wrong, but that the netmask is overwritten as soon as the IP address field blurs, and the previous PR doesn't change that. A user tweaking the last octet of their IP address and clicking Save will not have the opportunity to notice the changed netmask before it applies and they lose access. /24 is less likely to break things but people do use /25s and 10.x.x.x networks. And /64 is the smallest used subnet for IPv6, /48 is quite often given out by ISPs.
This PR also switches to checking
ipfor colons instead of the wholeaddressfield, but that's very minor as nothing's validated at this stage.Type of change
Example configuration
N/A
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: