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

mwan3 - json_load fails with some data #10533

Closed
wants to merge 1 commit into from

Conversation

yurtesen
Copy link
Contributor

In my system ubus -S call network.interface.wan status returns the following data:

{"up":true,"pending":false,"available":true,"autostart":true,"dynamic":false,"uptime":2437,"l3_device":"eth0.2","proto":"dhcp","device":"eth0.2","metric":30,"dns_metric":0,"delegation":true,"ipv4-address":[{"address":"84.248.177.219","mask":20}],"ipv6-address":[],"ipv6-prefix":[],"ipv6-prefix-assignment":[],"route":[{"target":"0.0.0.0","mask":0,"nexthop":"84.248.176.1","source":"84.248.177.219\/32"}],"dns-server":["193.210.18.18","193.210.19.19"],"dns-search":[],"inactive":{"ipv4-address":[],"ipv6-address":[],"route":[],"dns-server":[],"dns-search":[]},"data":{"hostname":"OpenWrt","leasetime":1200,"ntpserver":"192.89.123.26 192.89.123.230"}}

For some reason which I did not figure out, this cause json_load to return Failed to parse message data error.

The end result is mwan3 using wrong interface name... as explained here:
https://forum.openwrt.org/t/mwan3-interface-wan-is-offline-and-tracking-is-down

The fix is to simply put $status into quotes "$status". I found out that in all other OpenWRT scripts the data is inside quotes when calling json_load

Maintainer: me / @<github-user> (find it by checking history of the package Makefile)
Compile tested: (put here arch, model, OpenWrt version)
Run tested: (put here arch, model, OpenWrt version, tests done)

Description:

@yurtesen yurtesen changed the title Fix json_load fails with some data mwan3 - json_load fails with some data Nov 12, 2019
@neheb
Copy link
Contributor

neheb commented Nov 13, 2019

Honestly, the entire file should be ran through shellcheck.

@feckert
Copy link
Member

feckert commented Nov 13, 2019

@neheb I know that I have this on my backlog but I'm afraid I'll break something.
The change then needs an in-depth test.

@yurtesen
Copy link
Contributor Author

yurtesen commented Nov 13, 2019

@feckert adding quotes around a variable probably does not need an in-depth test :) Not having them is causing troubles for sure. I spent a good part of a day trying to figure out what was going on.

@feckert
Copy link
Member

feckert commented Nov 13, 2019

@yurtesen that was not for you. I wanted to say that I have this on my backlog to do a shellcheck run.

So to fix your problem I could merge this.
But could you please update also the PKG_RELEASE and PKG_VERSION
from

PKG_VERSION:=2.8.0
PKG_RELEASE:=2

to

PKG_VERSION:=2.8.1
PKG_RELEASE:=1

And also sign off your work and update the commit headline to `mwan3: ```

thanks

@yurtesen
Copy link
Contributor Author

@feckert I am assuming I have to undo the commit and then commit again with new headline and Signed-off-by line? is there any other way?

Also you probably prefer the fix and PKG_VERSION/RELEASE update in 2 different commits or? :)

I will be able to return back to this after few hours... I will have more time to figure out how to resolve this when I am back.

@feckert
Copy link
Member

feckert commented Nov 13, 2019

You could add this into one commit.
Workflow for this change:
Edit the makefile of mwan3 commit this.
After that do a rebase and stash the commits with a fix up.
After that to a commit --amend and update your commit message.
Then you are ready to push to this pullrequest branch with a push force.

Sometimes the return value of `ubus -S call network.interface.wan status`
cause `json_load` to return `Failed to parse message data` error.

To avoid this, the JSON data always should be quoted with double quotes.

Signed-off-by: Evren Yurtesen <eyurtese@abo.fi>
@yurtesen
Copy link
Contributor Author

@feckert I think I managed to do it.... was not fun... :D

@feckert
Copy link
Member

feckert commented Nov 14, 2019

@feckert I think I managed to do it.... was not fun... :D

The advantage is now you know how to do it for the future and you've learned something.
I update the heading so a pushed it directly 94e0c78 . So I close this pullrequest.
Thanks for your contribution 👍

@feckert feckert closed this Nov 14, 2019
@feckert
Copy link
Member

feckert commented Nov 14, 2019

I also pushed the fix to branch openwrt-19.07
84756e7

@yurtesen yurtesen deleted the patch-1 branch February 2, 2020 17:28
@BKPepe
Copy link
Member

BKPepe commented Apr 28, 2020

What about OpenWrt 18.06?

@feckert
Copy link
Member

feckert commented Apr 28, 2020

@BKPepe I thought this is EOL?
And I could not port this one by one.
There is a source difference on 18.06.

@BKPepe
Copy link
Member

BKPepe commented Apr 28, 2020

OpenWrt 18.06 is not End of Life, yet.
You can take a look here https://openwrt.org/docs/guide-developer/security

I think that mwan3 can be used by a higher amount of people and add double quotes shouldn't harm anyone and should save time while troubleshooting it.

@feckert
Copy link
Member

feckert commented Apr 28, 2020

@BKPepe thanks for the hint.
I am not using 18.06 anymore and i thought this is not necessary.
I pushed the fix with some changes baacda1 to openwrt-18.06 branch

@fabiodegaetano
Copy link

Hello,

thanks a lot for these precious info.
I have OpenWrt 18.06.1 r7258-5eb055306f / LuCI openwrt-18.06 branch (git-18.196.56128-9112198)

and I have currently the same problem described in this thread.

I have installed luci-app-mwan3 git-18.196.56128-9112198-1 a few weeks ago from these feeds

src/gz glinet_core https://fw.gl-inet.com/releases/kmod-3.1/ar71xx/nand
src/gz glinet_base https://fw.gl-inet.com/releases/packages-3.x/ar71xx/base
src/gz glinet_gli_pub https://fw.gl-inet.com/releases/packages-3.x/ar71xx/gli_pub
src/gz glinet_packages https://fw.gl-inet.com/releases/packages-3.x/ar71xx/packages
src/gz glinet_luci https://fw.gl-inet.com/releases/packages-3.x/ar71xx/luci
src/gz glinet_routing https://fw.gl-inet.com/releases/packages-3.x/ar71xx/routing
src/gz glinet_telephony https://fw.gl-inet.com/releases/packages-3.x/ar71xx/telephony
src/gz glinet_glinet https://fw.gl-inet.com/releases/packages-3.x/ar71xx/glinet

could you be so kind to tell me where I can find this fix?

thanks a lot

@feckert
Copy link
Member

feckert commented Jan 21, 2021

@fabiodegaetano baacda1

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 this pull request may close these issues.

None yet

5 participants