search

openwrt / luci

LuCI - OpenWrt Configuration Interface
Apache License 2.0
6.28k stars 2.51k forks source link

BUG: Misinformed URL buttons on "Unsaved Changes" page. Extra // in url on "Apply", "Revert", and "Apply & Save" Buttons. #961

Closed NvrBst closed 5 years ago

NvrBst commented 7 years ago

Not very crucial, but it's a quick and easy fix. The buttons on the "Unsaved Changes" page all have an extra "//" in them when you click them.

http://192.168.0.1/cgi-bin/luci//admin/uci/apply
http://192.168.0.1/cgi-bin/luci//admin/uci/saveapply
http://192.168.0.1/cgi-bin/luci//admin/uci/revert

The file here: _luci/modules/luci-mod-admin-full/luasrc/view/adminuci/changes.htm

<form class="inline" method="post" action="<%=controller%>/admin/uci/apply">
<form class="inline" method="post" action="<%=controller%>/admin/uci/saveapply">
<form class="inline" method="post" action="<%=controller%>/admin/uci/revert">

Removing the extra / does produce the correct URL with only a single slash. Unsure if that is the proper way to fix this.

If you look at the old bootstrap theme header.htm, it used to do the button similarily: <%=controller%>/<%=category%>/uci/changes, which produced an extra // as well now.

bootstrap theme changed the <%=controll.. code to a url function now, and used:

write('<a class="label notice" href="%s?redir=%s">%s: %d</a>' %{
                    url(category, 'uci/changes'),

Perhaps the proper way would be to use the function "url(category, 'admin/uci/apply')"? I donno. Either or an easy fix. All 3 buttons on the page need to be fixed, either delete the / before "admin" or change to the url(category... function. Should be quick.

danielfdickinson commented 7 years ago

@NvrBst While it's looks a little wonky it's actually a perfectly correct path according standards definitions; It's be nice for it to be right, however the issue is not really the piece of code you show, because the double-slash issues occurs through the codebase. I'm not sure if there is a single definition that could fix this, or of that would unexpectedly break something or other where you don't end up with the double-slashes but only a single slash (i.e. there may be inconsistent usage somewhere that breaks when you remove the slash, or remove it in the wrong location). Thorough testing would be needed to verify that a seemingly innocuous fix doesn't break things.

jow- commented 5 years ago

Should be fixed with recent versions