It's true that 18.06 dnsmasq prints an error message instead of crashing, but it fails to start up and procd eventually gives up, so you still need to be careful.
How will you supporting the IDN domains? I'm guessing using the idn
package for domains hitting your filter?
Pardon the delay -- I did have a chance to try out the changes but got sidetracked later, so let me address your two changes separately:
Filtering Improperly Encoded IDN Domains:
Yay, it now works! I see you rightly added character class filtering (e.g. [:alnum:]) to fix this. Be aware their behavior depends on locale, which you need to control for the solution to be robust. You should consider adding e.g. LC_ALL="C"
to your script, or the equivalent.
Download Failures - process_url()
, wget
It seemed to work better at first, but after several restart
or reload
attempts I could still trigger download errors. As both @dibdot and I mentioned before, it does seem like there's an underlying race or timing issue, which your change (below) maybe only tickled rather than fixed.
@@ -201,3 +203,3 @@
done
- if ! touch "$R_TMP" || ! $dl_command "$1" -O "$R_TMP" 2>/dev/null; then
+ if ! $dl_command "$1" -O "$R_TMP" 2>/dev/null || [ ! -s "$R_TMP" ]; then
output 2 "[DL] $type $label $__FAIL__\n"
I also tried disabling backgrounding the calls to process_url()
, and that has worked reliably well, so perhaps consider adding an option to do this?
I also noticed the DNS firewall rule isn't being removed properly, which is a big problem for anyone trying to manage their own VPN/DNS/redirect scheme. You can see for yourself: disable DNS redirect in the GUI, or even stop the service, and the rule persists in both ubus
and iptables
(and if you investigate further you'll find other underlying serious issues). Aside from that, can I suggest a UCI default of '0' for force_dns
, which is less likely to cause problems "out of the box"?
Cheers!