search

ExtremeFiretop / MerlinAutoUpdate-Router

Merlin(A)uto(U)pdate is a Merlin router script which allows you to remotely identify a stable firmware update for an ASUS Merlin router, and automatically download and update via an unattended method directly from the router.
https://www.snbforums.com/threads/merlinau-v1-2-7-the-ultimate-firmware-auto-updater-amtm-addon.91326/
GNU General Public License v3.0
15 stars 1 forks source link

_ValidatePrivateIPv4Address_ Bug Fix #263

Closed ExtremeFiretop closed 2 months ago

ExtremeFiretop commented 2 months ago

As discussed in the GNUton branch here:

https://github.com/ExtremeFiretop/MerlinAutoUpdate-Router/pull/186#issuecomment-2211252599

This change is to address a bug discovered with the ValidatePrivateIPv4Address function.

Martinski4GitHub commented 2 months ago

@ExtremeFiretop,

Well, for some reason, my comment was flagged as "Spammy" so that might be why it's not visible to you.

PR#263_CommentMarkedAsSpammy

I don't know why or how to "fix" the situation to remove the "Spammy" mark.

Martinski4GitHub commented 2 months ago

@ExtremeFiretop,

Just to be sure here's my comment:

The changes look good, but on line 5280 there’s no need to reverse the RegEx rules. I know this falls in the “subjective” category, but when parsing a numerical range, it’s considered a best-practice coding standard to start with the smallest valid number and work your way up to the largest valid number within the range. So the simplest change for the fix would be:

readonly IPv4octet_RegEx="([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])"

This makes it easier to read, especially if someone else is not used to reading RegEx rules, and passes our Linter tool coding standard checks.

My 2 cents.