Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

luci-app-firewall: cannot choose Any zone in Output zone field | temporary fix found #7591

Closed
1 task done
this-username-has-been-taken opened this issue Jan 29, 2025 · 11 comments · Fixed by #7592
Closed
1 task done

Comments

@this-username-has-been-taken
Copy link
Contributor

this-username-has-been-taken commented Jan 29, 2025

Is there an existing issue for this?

  • I have searched among all existing issues (including closed issues)

screenshots or captures

Hello!

I have just build and installed a fresh firmware from the master/main branch and faced with an issue: I cannot add or modify a firewall rule and set Output zone to Any zone: LuCI shows an error: Expecting: valid UCI identifier. Please refer to the attached screenshot for more details.

It is possible to modify the rule via editing the config file, but that's not very convenient.

I have searched the similar issues and found this: #7563
I believe my issue might be connected with it.

UPD - temporary fix:

Quick and dirty fix for a live system - thanks to @systemcrash post here: #7563 (comment)

  1. SSH to the router
  2. Edit file /www/luci-static/resources/tools/widgets.js, e.x. nano /www/luci-static/resources/tools/widgets.js
  3. Delete the following substring: datatype:L.hasSystemFeature('firewall4')?(this.multiple?'list(uciname)':'uciname'):this.multiple?'list(and(uciname,maxlength(11)))':'and(uciname,maxlength(11))',
  4. Save file (ctrl+o).
  5. Refresh the LuCI interface.
  6. Done. This fix should work until a better and permanent solution is introduced.

Image

Actual behaviour

When adding or modifying a firewall rule it is impossible to set Output zone field to Any zone value. LuCI shows an error: Expecting: valid UCI identifier.

Expected behaviour

When adding or modifying a firewall rule it is possible to set Output zone field to Any zone value. LuCI accepts the value and saves it in the config file without any errors.

Steps to reproduce

  1. Obtain a freshly built (preferably based on master/main branch) firmware and install it.
  2. Log in into LuCI.
  3. Navigate to Network -> Firewall -> Traffic rules.
  4. Add a new rule and set Output zone field to Any zone.
  5. Observe the issue: LuCI highlights the value with red and shows Expecting: valid UCI identifier error. It is impossible to save the rule

Additional Information

{
        "kernel": "6.6.74",
        "hostname": "router",
        "system": "ARMv8 Processor rev 0",
        "model": "Bananapi BPI-R4",
        "board_name": "bananapi,bpi-r4",
        "rootfs_type": "squashfs",
        "release": {
                "distribution": "OpenWrt",
                "version": "SNAPSHOT",
                "revision": "r28724-807074309d",
                "target": "mediatek/filogic",
                "description": "OpenWrt SNAPSHOT r28724-807074309d",
                "builddate": "1738094344"
        }
}

What browsers do you see the problem on?

Firefox

Relevant log output

@this-username-has-been-taken this-username-has-been-taken changed the title luci-app-firewall: cannot choose Any zone in Output zone field luci-app-firewall: cannot choose Any zone in Output zone field | temporary fix found Jan 29, 2025
@systemcrash
Copy link
Contributor

Hmm. Evidently there remain edge cases for the widget. My guess is that the name with a space in it is the trigger here.

Any suggestions @adelton ?

@systemcrash
Copy link
Contributor

@this-username-has-been-taken I wonder whether you can test whether the following works:

			datatype: L.hasSystemFeature('firewall4')
				? ( this.multiple ? 'list(string)' : 'string' )
				: this.multiple ? 'list(and(string,maxlength(11)))' : 'and(string,maxlength(11))',

It's not ideal, but at least fixes this edge-case, I think (assuming string permits the space in this 'name').

@this-username-has-been-taken
Copy link
Contributor Author

this-username-has-been-taken commented Jan 29, 2025

@systemcrash , just checked: yes, that worked!
With that code it is now possible to choose Any zone as well as "real" zones:

Image

Image

But I don't remember if I was able to delete a chosen Output zone or not before. Now if I choose zone once - I won't be able to delete it: both without datatype section and with your suggestion.

@systemcrash
Copy link
Contributor

But I don't remember if I was able to delete a chosen Output zone or not before

Delete?

@adelton
Copy link
Contributor

adelton commented Jan 29, 2025

@systemcrash Is it expected that when I install OpenWrt 24.10.0-rc7 (r28417-daef29c75d) and upgrade all upgradable packages, giving me

# opkg list-installed | grep luci | sort
liblucihttp-ucode - 2023.03.15~9b5b683f-r1
liblucihttp0 - 2023.03.15~9b5b683f-r1
luci - 25.027.83426~170375e
luci-app-firewall - 25.027.83426~170375e
luci-app-package-manager - 25.027.83426~170375e
luci-base - 25.027.83426~170375e
luci-light - 25.027.83426~170375e
luci-mod-admin-full - 25.027.83426~170375e
luci-mod-network - 25.027.83426~170375e
luci-mod-status - 25.027.83426~170375e
luci-mod-system - 25.027.83426~170375e
luci-proto-ipv6 - 25.027.83426~170375e
luci-proto-ppp - 25.027.83426~170375e
luci-ssl - 25.027.83426~170375e
luci-theme-bootstrap - 25.027.83426~170375e
rpcd-mod-luci - 20240305-r1

that I don't reproduce that issue?

In fact,

root@OpenWrt:~# grep firewall4 /www/luci-static/resources/tools/widgets.js

does not find anything.

Does it mean we don't have a package which would contain #7564? I'm trying to establish a baseline for a reproducer (and a fix).

@this-username-has-been-taken
Copy link
Contributor Author

Delete?

I meant that when you open a new rule dialog the Output zone is empty:

Image

If you save a rule like this (with an empty value - not choosing anything) - the zone value will be set to this device.
After you choose something, you won't be able to make it empty again, i.e. it won't be possible to set this device value

@adelton
Copy link
Contributor

adelton commented Jan 29, 2025

@this-username-has-been-taken: Could you check the code in #7592?

@adelton
Copy link
Contributor

adelton commented Jan 29, 2025

As for removing the selection completely after either a zone or Any zone has been selected: when I modify the code to drop the datatype completely, bringing the code to the state before a075566, I'm not able to completely clear the selection either. That would suggest that it was not possible to clear the selection even with the previous versions (23.05.5) so that behaviour is not a regression.

@this-username-has-been-taken
Copy link
Contributor Author

this-username-has-been-taken commented Jan 29, 2025

@adelton , I have just checked: the code from #7592 works!

I'd suggest putting "else" part under parentheses too to make it look uniform, but that's not critical:

datatype: L.hasSystemFeature('firewall4')
				? ( this.multiple ? 'list(or(uciname,"*"))' : 'or(uciname,"*")' )
				: ( this.multiple ? 'list(or(and(uciname,maxlength(11)),"*"))' : 'or(and(uciname,maxlength(11)),"*")' ),

As per this device value - it works either! You just have to choose anything besides Device (output) in the Source zone field - and viola - after that Device (input) value will appear in the Destination zone field. I forgot about that.

@systemcrash
Copy link
Contributor

@systemcrash Is it expected that when I install OpenWrt 24.10.0-rc7 (r28417-daef29c75d) and upgrade all upgradable packages, giving me

Your PR was merged to main (snapshots) but not to 24. I figured some edge-cases might still be lingering.

@this-username-has-been-taken
Copy link
Contributor Author

Thank you very much for a quick fix!!! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants