Skip to content

Comments

Fix Web-Access Restrictions, Changelog Verification, WebUI Selections, and Firmware Run Estimates#476

Merged
Martinski4GitHub merged 22 commits intodevfrom
Fix-Web-Access-Restrictions
Jun 1, 2025
Merged

Fix Web-Access Restrictions, Changelog Verification, WebUI Selections, and Firmware Run Estimates#476
Martinski4GitHub merged 22 commits intodevfrom
Fix-Web-Access-Restrictions

Conversation

@ExtremeFiretop
Copy link
Owner

@ExtremeFiretop ExtremeFiretop commented May 19, 2025

Things Fixed in this PR:

- All issues reported by: kriukas

1. Added missing changelog patch to force 3006 in an upgrade from 3004 to 3006

  • Changes made within the ChangelogVerificationCheck function which was previously missed
  • We don't want to make the jump only when viewing the changelog, but also the verification makes the jump to the right changelog

2. Fix Changelog Verification for firmware going from 3004 to 3006.
image

  • Previously only checked between 2 matching firmware versions, (currently installed and update version) which won't exist between firmware changelogs in upgrades from 388.8 and 3006.
  • Also previously does not flatten the content so line breaks are missed such as:
    "any additionnal GN must be
    manually reconfigured."
    Found here: https://www.asuswrt-merlin.net/changelog-3006

3. Fix Web Access Restrictions by modifying the regex to accept subnets larger than /20
image

  • Steps to reproduce:
  • Set the Web Access Restrictions to 172.29.0.0/12 and attempt to set the password in the shell script.
  • Next; set multiple large overlapping ranges such as: 172.16.0.0/12, 169.29.0.0/5, 192.168.50.0/24 and attempt to set the password in the shell script.

4. Previously changing the postpone period in the WebUI did not correctly recalculate the new estimate flash time. Now we force it too.
image

  • Steps to reproduce:
  • Set your router to 1 version behind in nvram; and start MerlinAU to detect an update available
  • Go to the WebUI and change the postpone period
  • It will not reflect
  • Reason: This is due to the fact that the calculation only currently runs once after an update is detected (if FW_New_Update_Expected_Run_Date is TBD or Empty)
  • However if a calculation has run already once when an update is detected, FW_New_Update_Expected_Run_Date won't be TBD or empty, we want it to recalculate anyways.

5. Set "Firmware Run Estimates" to TBD when disabling "built-in firmware update checks"
image

  • Also resolved issue number 6 below

6. Fixed emails showing an estimated date when the "Enable Automatic F/W Update Checks" is DISABLED
image

  • Solution:
  • Set "Firmware Run Estimates" to TBD when disabling "built-in firmware update checks" (number 5)

- My own discovery which I fixed as well:

1. Fix the WebUI Password Focus on Edge/Chrome, which is stealing focus even when the password is valid
image

  • Steps to reproduce:
  • Open our WebUI in Chrome or Edge, and try to click any of the text fields to edit (such as the postpone period)
  • Workaround:
  • Click on the expand/collapse to restore the ability to fill in text fields

Fix Web Access Restrictions Regex
Add Missing Changelog Patch
Missing Comment Updates
@ExtremeFiretop ExtremeFiretop added the bug Something isn't working label May 19, 2025
Actually Fix Web Access Restrictions
Currently does not flatten the content so word wraps are missed such as:

"any additionnal GN must be
          manually reconfigured."

Also only checks between 2 matches firmware versions, which won't exist between firmware jumps in changelog 388.8 and 3006.

This commit addresses these both by flattening the new lines and also checking for the date format instead of the  firmware versions.
Set Estimates to TBD when disabling
Previously changing the postpone period in the WebUi did not correctly recalculate the new estimate flash time. Now we force it too.

As well previously the estimate did not reset to (TBD) when disabling update checks.
Making it an optional parameter instead
@ExtremeFiretop ExtremeFiretop marked this pull request as ready for review May 22, 2025 01:38
@ExtremeFiretop ExtremeFiretop changed the title Fix Web Access Restrictions Fix Web Access Restrictions and Many More May 22, 2025
Minor Fixes to Fix the Invalid Password Focus on Edge/Chrome

Currently on Edge/Chrome; even when the password is validated; the user is unable to click elsewhere like to change the postpone period. This fixes that issue.
Fix Memory Clearing for Low Memory Models
Adding Back conn_diag
Re-Implementing Memory Diff Solution from 0993ac5
Removing Memory Subtraction Solution
Lowering overhead by half from 50% to 25%
@ExtremeFiretop
Copy link
Owner Author

@Martinski4GitHub

Any chance we can review this one in the coming days this weekend? :D
If we don't get any feedback from the user in the open issue I'll just rollback the overhead change

@Martinski4GitHub
Copy link
Collaborator

@Martinski4GitHub

Any chance we can review this one in the coming days this weekend? :D

Yes, I think over this weekend I'll have the time (finally!!), especially since the work related to the AMTM-OSR migration and production release announcements is virtually over for now (only uiScribe remaining, but there's no rush).

My wife and I have plans to visit her parents on Saturday afternoon, so I'll perform the 3006.102.x F/W Update on their RT-AX86U_PRO router and reconfigure it for their home network. Having already done this for my own parents' router, this should not take very long because their home network is also fairly simple.

If we don't get any feedback from the user in the open issue I'll just rollback the overhead change

I have an idea I'd like to explore about that, but I'll do it after the PR review.
Have a good night, bud!!

@ExtremeFiretop
Copy link
Owner Author

@Martinski4GitHub

Considering you say you have some ideas and the user is around to test your feedback. I'll remove the changes from this PR and let you handle it in another PR.

I've also greatly increased the detail of all the bug reports above.

Undoing Memory Based Changes
@ExtremeFiretop ExtremeFiretop changed the title Fix Web Access Restrictions and Many More Fix Web-Access Restrictions, Changelog Verification, WebUI Selections, and Firmware Run Estimates May 31, 2025
@Martinski4GitHub
Copy link
Collaborator

@Martinski4GitHub

Considering you say you have some ideas and the user is around to test your feedback. I'll remove the changes from this PR and let you handle it in another PR.

I've also greatly increased the detail of all the bug reports above.

Great!!! I'm starting to review the PR. Hopefully, you're no longer making any code changes while I review. :>)

@ExtremeFiretop
Copy link
Owner Author

@Martinski4GitHub
Considering you say you have some ideas and the user is around to test your feedback. I'll remove the changes from this PR and let you handle it in another PR.
I've also greatly increased the detail of all the bug reports above.

Great!!! I'm starting to review the PR. Hopefully, you're no longer making any code changes while I review. :>)

Last change was the readme 😜
Like I mentioned I've left the memory stuff out of this PR for now so it's only the listed stuff.

No additional changes incoming

@ExtremeFiretop
Copy link
Owner Author

@Martinski4GitHub

Going to take a quick dinner break but I'll be back in 20 minutes or so.
Take your time and let me know if you got questions, I hope my increased detail helps outline the issues better!! ☺️

Copy link
Collaborator

@Martinski4GitHub Martinski4GitHub left a comment

Choose a reason for hiding this comment

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

Approved.
There's a couple of fixes I need to make, but overall it's looking good.

P.S.
I'll submit the fixes in a separate PR, and I'll also start to implement my idea to address the RT-AX86S router issue.

@Martinski4GitHub Martinski4GitHub merged commit 6ad695c into dev Jun 1, 2025
2 checks passed
@ExtremeFiretop
Copy link
Owner Author

Approved. There's a couple of fixes I need to make, but overall it's looking good.

P.S. I'll submit the fixes in a separate PR, and I'll also start to implement my idea to address the RT-AX86S router issue.

I'm around no worries looking forwards!
Was the additional fixes related to the WebUI or the Web Access Restrictions or both? Hehehe

@Martinski4GitHub
Copy link
Collaborator

Just FYI,
My wife wants us to make an "ice cream stop" so I'll be back in about 30 minutes.
Warm Summer days have already started around here, :>).

@Martinski4GitHub
Copy link
Collaborator

Approved. There's a couple of fixes I need to make, but overall it's looking good.
P.S. I'll submit the fixes in a separate PR, and I'll also start to implement my idea to address the RT-AX86S router issue.

I'm around no worries looking forwards! Was the additional fixes related to the WebUI or the Web Access Restrictions or both? Hehehe

Web Access Restrictions...

@ExtremeFiretop
Copy link
Owner Author

Just FYI, My wife wants us to make an "ice cream stop" so I'll be back in about 30 minutes. Warm Summer days have already started around here, :>).

I have an ice cream sandwich in hand! The trees are green, it's nice this time of year I totally understand 🤣

@ExtremeFiretop
Copy link
Owner Author

Approved. There's a couple of fixes I need to make, but overall it's looking good.
P.S. I'll submit the fixes in a separate PR, and I'll also start to implement my idea to address the RT-AX86S router issue.

I'm around no worries looking forwards! Was the additional fixes related to the WebUI or the Web Access Restrictions or both? Hehehe

Web Access Restrictions...

Fantastic! As I mentioned on SNB, your free to replace anything if it's gross 🤢

@Martinski4GitHub
Copy link
Collaborator

Martinski4GitHub commented Jun 1, 2025

Approved. There's a couple of fixes I need to make, but overall it's looking good.
P.S. I'll submit the fixes in a separate PR, and I'll also start to implement my idea to address the RT-AX86S router issue.

I'm around no worries looking forwards! Was the additional fixes related to the WebUI or the Web Access Restrictions or both? Hehehe

Web Access Restrictions...

Fantastic! As I mentioned on SNB, your free to replace anything if it's gross 🤢

Nothing gross like that, LOL!!!

Basically, the main issue was that the regular expressions for the Main LAN IP address and the CIDR IP address subnet blocks were too wide and permissive. They allow entries like "10.0.0.0/4", which means half the internet was allowed access, LOL!!!

For a private LAN IP address, a valid CIDR block can be only within a 24-bit subnet block, at most.
Examples:
10.0.0.0/8
172.16.0.0/12
192.168.0.0/16

If the CIDR IP address subnet block has a mask less than 8, you start to cross boundaries into the wider internet (i.e. public WAN IP addresses). If a user indeed wants access over WAN, then he should enter a distinct line with the public WAN IP, but that's separate from allowing access to the main private LAN IP subnet.

P.S.
I know that the WebUI Access Restrictions allow pretty much any entry, so users can shoot themselves in the foot (and in the head) if they are not careful. But I think at least our code should give a regular warning when the CIDR block is too wide for the private LAN IP address.

@ExtremeFiretop ExtremeFiretop deleted the Fix-Web-Access-Restrictions branch June 1, 2025 11:33
@ExtremeFiretop
Copy link
Owner Author

Approved. There's a couple of fixes I need to make, but overall it's looking good.
P.S. I'll submit the fixes in a separate PR, and I'll also start to implement my idea to address the RT-AX86S router issue.

I'm around no worries looking forwards! Was the additional fixes related to the WebUI or the Web Access Restrictions or both? Hehehe

Web Access Restrictions...

Fantastic! As I mentioned on SNB, your free to replace anything if it's gross 🤢

Nothing gross like that, LOL!!!

Basically, the main issue was that the regular expressions for the Main LAN IP address and the CIDR IP address subnet blocks were too wide and permissive. They allow entries like "10.0.0.0/4", which means half the internet was allowed access, LOL!!!

For a private LAN IP address, a valid CIDR block can be only within a 24-bit subnet block, at most. Examples: 10.0.0.0/8 172.16.0.0/12 192.168.0.0/16

If the CIDR IP address subnet block has a mask less than 8, you start to cross boundaries into the wider internet (i.e. public WAN IP addresses). If a user indeed wants access over WAN, then he should enter a distinct line with the public WAN IP, but that's separate from allowing access to the main private LAN IP subnet.

P.S. I know that the WebUI Access Restrictions allow pretty much any entry, so users can shoot themselves in the foot (and in the head) if they are not careful. But I think at least our code should give a regular warning when the CIDR block is too wide for the private LAN IP address.

I see what your saying that makes sense. Yes I agree.
I was more focused on making it work with whatever could be entered. But I totally see your point, that just because you can doesn't mean you should, and in the case of Web Access Restrictions that's especially true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants