luci icon indicating copy to clipboard operation
luci copied to clipboard

luci-ddns writes incorrect service_name when "--custom--" service is chosen

Open wonson opened this issue 3 years ago • 5 comments

luci-ddns write incorrect service into /etc/config/ddns when "--custom--" service is chosen option service_name '-' where this line should be deleted or commented ( https://openwrt.org/docs/guide-user/services/ddns/client#custom_service1 )

This would result in "No update_url found/defined or no update_script found/defined! - TERMINATE" error when it try to update

wonson avatar Mar 16 '21 14:03 wonson

confirmed, same issue. Deleting the line corrects the problem.

plmarcus avatar Oct 26 '21 01:10 plmarcus

The problem is this line. https://github.com/openwrt/luci/blob/master/applications/luci-app-ddns/htdocs/luci-static/resources/view/ddns/overview.js#L517 Please change this line from service_name.value('-',"-- " + _("custom") + " --"); to service_name.value('',"-- " + _("custom") + " --");

Please report back if this solve the issue.

Maybe we also need an service_name.rmempty = true; there?

feckert avatar Oct 26 '21 08:10 feckert

I was having a look at the code and I think your solution above is not ideal. From what I can understand, we need to suppress the service_name='-' ONLY when saving the configuration file.

So, that is when we call the uci.set command on service_name
Here

uci.set('ddns',section_id,'service_name',service_value);

As it is already done in other parts of the code, we need to check if it is a custom script and then IGNORE the uci.set command for the service_name.

We can do that with

if (service_value != '-') { uci.set('ddns',section_id,'service_name',service_value); }

Was quite the pain to test this in a running OpenWRT router. Browser cache also interfering.

Also, this is done when you select the service type and click CREATE SERVICE (pre-creation)


Patching existing install

(EDIT) Because I had to do the same thing in other routers, I made this one-liner that uses sed to patch it. Should be safe, because if anything in the long pattern changes, it will do nothing. Use it at your own risk tho.

SRC="return m.save(function(){uci.add('ddns','service',section_id);uci.set('ddns',section_id,'service_name',service_value);uci.set('ddns',section_id,'use_ipv6',ipv6_value);})" && DST="return m.save(function(){uci.add('ddns','service',section_id);if(service_value!='-'){uci.set('ddns',section_id,'service_name',service_value);}uci.set('ddns',section_id,'use_ipv6',ipv6_value);})" && sed -i "s/$SRC/$DST/g" /www/luci-static/resources/view/ddns/overview.js


This is what the sed command does:

File: /www/luci-static/resources/view/ddns/overview.js

Original:

return m.save(function(){uci.add('ddns','service',section_id);uci.set('ddns',section_id,'service_name',service_value);uci.set('ddns',section_id,'use_ipv6',ipv6_value);})

Changed:

return m.save(function(){uci.add('ddns','service',section_id);if(service_value!='-'){uci.set('ddns',section_id,'service_name',service_value);}uci.set('ddns',section_id,'use_ipv6',ipv6_value);})

Reload uhttp

/etc/init.d/uhttpd reload

Your browser may be stubborn about the Javascript cache though. Try in another browser to be sure (or inspect the source).

I think this is a simple working solution. I probably won't be able to make a PR any time soon.

Cheers!

Gus

tavinus avatar Mar 31 '22 05:03 tavinus

Sorry to double-post, but this is something else kind of related.

I noticed that when creating the custom service, you will have to select some other service and then select the -- custom -- service back for it to work. I think there used to be ANOTHER first option with a description before the custom one (am I wrong?). This may be related to removing that? Maybe some code that used to compensate for that?

This is how it behaves image

Then you click something else image

Then -- custom -- works image


EDIT

Am I wrong to think there should be no plural in the sentence

Add new services...

Honestly, I only thought about that because I read it 50 times in 1 minute when posting this (and after it, revising it).

This is probably better

Add New DDNS Service

tavinus avatar Apr 01 '22 02:04 tavinus

FWIW, i was bitten by this just now.

attila-lendvai avatar Apr 03 '22 18:04 attila-lendvai