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.

Sign up for GitHub

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

Jump to bottom

ESP crashes with {} in custom dashboard #3873

Closed
V0JT48 opened this issue Dec 11, 2021 · 12 comments · Fixed by #3874
Closed

ESP crashes with {} in custom dashboard #3873

V0JT48 opened this issue Dec 11, 2021 · 12 comments · Fixed by #3874
Labels
Category: Frontend Related to web-interface Category: Stabiliy Things that work, but not as long as desired Status: Fixed Commit has been made, ready for testing Type: Bug Considered a bug

Comments

@V0JT48
Copy link

V0JT48 commented Dec 11, 2021

Steps already tried...

  • [ x] Tried a clean install (empty .bin files are included in the ZIP)
  • [ x] Tested previous/other build (mention which one already tested)
  • [ x] Tested on other node to make sure hardware isn't defective.
  • [NA] Verified if the problem is limited to a single plugin/controller

Summarize of the problem/feature request

Device reboots if custom dashboard source file contains {}

Expected behavior

YOUR TEXT GOES HERE

Actual behavior

After upgrading to ESP_Easy_mega_20210802_normal_ESP8266_4M1M my custom dashboard stopped working and device resets when *.esp is accessed. Same results are with ESP_Easy_mega_20211105_normal_ESP8266_4M1M. It worked fine with ESP_Easy_mega-20200410_normal_ESP8266_4M1M and 20200801 release.
I tried to narrow down what part of my HTML code causes it. So far main issue seems to be related to curly braces but not all appearances. The simplies example that triggers the issue is below.
I have some javascript code in .esp files and this makes it impossible to use them.

Steps to reproduce

Upload file test.esp and access it http://ESP_IP/test.esp
Content of test.esp is below.

Rebooting or powering off produces same result.

System configuration

Rules and NTP enabled
Hardware: ESP12E

ESP Easy version: ESP_Easy_mega_20210802_normal_ESP8266_4M1M, ESP_Easy_mega_20211105_normal_ESP8266_4M1M

Rules or log data

<!DOCTYPE html><meta name="viewport" content="width=width=device-width, initial-scale=1">
<HTML><HEAD><meta charset='utf-8'/></HEAD><BODY>
{}
</BODY></HTML>
@TD-er TD-er added Category: Frontend Related to web-interface Category: Stabiliy Things that work, but not as long as desired Type: Bug Considered a bug labels Dec 11, 2021
@TD-er
Copy link
Member

TD-er commented Dec 11, 2021 •
edited

Hmm I just took a quick look at the function to handle the custom page.
The main suspect so far seems to be the parseTemplate function.
But that would mean it would probably not only crash on parsing the dashboard file, but also lots of other places where this is called.
e.g. when used on a display (like Framed OLED) or in rules or generating log entries.

I will try to reproduce it here too with a debugger attached.

It would be great to know if there are other builds between this quite large range of builds you tested to see where it may have been broken.

@TD-er
Copy link
Member

TD-er commented Dec 11, 2021

It seems like the ESP is then entering some infinite loop.
What kind of crash do you see? Does it actually crash, or can to press reset to get out of it?

@V0JT48
Copy link
Author

V0JT48 commented Dec 11, 2021

The deice reboots almost instantly. I have not tried it with serial port attached if some more details are visible.
After some experimenting possible workaround for javascrip is to remove all whitespace at begining of lines. Code is not so readable but page loads.

@TD-er
Copy link
Member

TD-er commented Dec 11, 2021

I tried your sample code on an ESP32 and it keeps "running" or at least is not rebooting.
The page seems to still be loading in the browser but noting seems to happen.

Also the ESP is no longer responsible until I reboot.

I think it may be a bug in get_next_inner_bracket, but still looking into it.

@TD-er
Copy link
Member

TD-er commented Dec 11, 2021

Yep the bug is in there.
Now have to find a proper work-around to make sure the {} is still removed.

@V0JT48
Copy link
Author

V0JT48 commented Dec 11, 2021

{} is just the simpliest example.
Crash:

if (x){
 a
} else {
 b
}

No crash:

if (x){
a
} else {
b
}

TD-er added a commit to TD-er/ESPEasy that referenced this issue Dec 11, 2021
@TD-er
[dashboard] Fix crash on parsing empty command {} (letscontrolit#3873)
404b7af 
Fixes: letscontrolit#3873
@TD-er TD-er mentioned this issue Dec 11, 2021
Merged
@TD-er
Copy link
Member

TD-er commented Dec 11, 2021

Can you test the fix I made here?
#3874

@V0JT48
Copy link
Author

V0JT48 commented Dec 11, 2021

Well it fixes the crash for {} but also removes it from HTML code which is also valid for evrything inbetween for cases it used to crash.

if (x){
 a
} else {
 b
}

Turns into

if (x) else

Code stays with removed whitespace at begining of line. Hard to tell which option is better.

@TD-er
Copy link
Member

TD-er commented Dec 11, 2021

It is a simple fix to leave the {} in the text.
Will make it after I put my daughter to bed.

TD-er added a commit to TD-er/ESPEasy that referenced this issue Dec 11, 2021
@TD-er
[dashboard] Fix parsing {} and leaving the braces when not a command
079744f 
See: letscontrolit#3873 (comment)
@TD-er
Copy link
Member

TD-er commented Dec 11, 2021 •
edited

Just pushed a new commit which should be the best of both world:

  • No crash (seems to be a nice feature, so left it in)
  • Leave {} when it contains texts which cannot be matched to a command.

@TD-er TD-er added the Status: Fixed Commit has been made, ready for testing label Dec 11, 2021
@V0JT48
Copy link
Author

V0JT48 commented Dec 11, 2021

It works great, thanks a lot for super fast fix.

@TD-er
Copy link
Member

TD-er commented Dec 11, 2021

Great!
Thanks for testing.

Will merge it now and it will be included in the next build. (hopefully tomorrow)

TD-er added a commit that referenced this issue Dec 11, 2021
@TD-er
Merge pull request #3874 from TD-er/bugfix/parse_empty_command
2556826 
[dashboard] Fix crash on parsing empty command {} (#3873)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Category: Frontend Related to web-interface Category: Stabiliy Things that work, but not as long as desired Status: Fixed Commit has been made, ready for testing Type: Bug Considered a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[dashboard] Fix crash on parsing empty command {} (#3873) TD-er/ESPEasy
2 participants
@TD-er @V0JT48