openshwprojects / OpenBK7231T_App

Open source firmware (Tasmota/Esphome replacement) for BK7231T, BK7231N, BL2028N, T34, XR809, W800/W801, W600/W601, BL602 and LN882H
https://openbekeniot.github.io/webapp/devicesList.html
1.49k stars 279 forks source link

JSON Status crashes for Staus 1,3 etc in http_tasmota_json_status_generic - possible solution included #1076

Closed MaxineMuster closed 8 months ago

MaxineMuster commented 9 months ago

Describe the bug When trying to get status with NTP enabled, request crashes device. This seems to be inside file src/httpserver/json_interface.c in

static int http_tasmota_json_status_generic(void* request, jsonCb_t printer)

in this section

       struct tm* ltm;
       int ntpTime = NTP_GetCurrentTime() - g_secondsElapsed;
       ltm = gmtime((time_t*)&ntpTime);

       if (ltm != 0) {
               printer(request, "\"StartupUTC\":\"%04d-%02d-%02dT%02d:%02d:%02d\",", ltm->tm_year + 1900, ltm->tm_mon + 1, ltm->tm_mday, ltm->tm_hour, ltm->tm_min, ltm->tm_sec);
       }

Firmware:

To Reproduce Steps to reproduce the behavior: Try to get page "http:///cm?cmnd=STATUS%203" leads to a crash in my case

works fine e.g. with "http:///cm?cmnd=STATUS%204"

I could trace it down to the conversion of time for the JSON result.

I suggest this a possible solution, at least its working for me now ;-):

diff --git a/src/httpserver/json_interface.c b/src/httpserver/json_interface.c
index 637ec835..1b0f92f1 100644
--- a/src/httpserver/json_interface.c
+++ b/src/httpserver/json_interface.c
@@ -666,15 +666,18 @@ static int http_tasmota_json_status_generic(void* request, jsonCb_t printer) {
        JSON_PrintKeyValue_String(request, printer, "OtaUrl", "https://github.com/openshwprojects/OpenBK7231T_App/releases/latest", true);
        JSON_PrintKeyValue_String(request, printer, "RestartReason", "HardwareWatchdog", true);
        JSON_PrintKeyValue_Int(request, printer, "Uptime", g_secondsElapsed, true);
-       struct tm* ltm;
-       int ntpTime = NTP_GetCurrentTime() - g_secondsElapsed;
-       ltm = gmtime((time_t*)&ntpTime);
-
-       if (ltm != 0) {
-               printer(request, "\"StartupUTC\":\"%04d-%02d-%02dT%02d:%02d:%02d\",", ltm->tm_year + 1900, ltm->tm_mon + 1, ltm->tm_mday, ltm->tm_hour, ltm->tm_min, ltm->tm_sec);
-       }
-       else {
+       if (NTP_IsTimeSynced()) {
+               struct tm* ltm;
+               time_t ntpTime = (time_t)(NTP_GetCurrentTime() - g_secondsElapsed);
+               
+               ltm = gmtime(&ntpTime);
+
+               if (ltm != 0) {
+                       printer(request, "\"StartupUTC\":\"%04d-%02d-%02dT%02d:%02d:%02d\",", ltm->tm_year + 1900, ltm->tm_mon + 1, ltm->tm_mday, ltm->tm_hour, ltm->tm_min, ltm->tm_sec);
+               }
+               else {

+               }
        }
        JSON_PrintKeyValue_Int(request, printer, "Sleep", 50, true);
        JSON_PrintKeyValue_Int(request, printer, "CfgHolder", 4617, true);
openshwprojects commented 9 months ago

Interesting, what is the cause of crash here? It seems to work for me on BK7231T Build on Jan 29 2024 03:41:26 version 1.17.433

MaxineMuster commented 9 months ago

I'm not really sure, the device just stops responding when requesting "status 3": no ping, nothing.

First I thought it would happen if NTP is not synced (so ntpTime would be negative), so I added the "if" around this part. But then it worked only with NTP not synced (sure, the code wasn't used ;-) in this case), but not, if it is synced - and in this case ntpTime would be positive. I didn't use debugger or some deeper inspection, I only thought it might be related to ntpTime as an int when NTP_GetCurrentTime() is an unsigned int, for I thought of ntp EPOCH starting 1900 - I know, its adjusted to 1970 so it should work o.k up to 2038, but I only realized that later. So I just was looking at the similar code of drv_bl_shared.c, which I knew was working, so I tried to adapt it and it worked.

And now I have a working version, but still don't know, what was the cause of the problem?!?

MaxineMuster commented 9 months ago

Could you please take a deeper look? Is it really "working", or just "not crashing"?

e.g.: If your time is synced, do you see "StartupUTC" in the JSON output?

If I try it on a sample program, it would expect, it's not shown.

#include <stdio.h>      // for puts
#include <time.h>       // for time, time_t, struct tm,  ... 

int main ()
{
       char buffer [255];
       struct tm* ltm;
//     just use a fixed epoch from today as "ntpTime"
       int ntpTime = 1707839364;

// working for me:
       time_t nt = (time_t)(ntpTime);
       ltm = gmtime(&nt);

       puts("Testing version 1");
       if (ltm != 0) {
          sprintf(buffer,"\"StartupUTC\":\"%04d-%02d-%02dT%02d:%02d:%02d\",", ltm->tm_year + 1900, ltm->tm_mon + 1, ltm->tm_mday, ltm->tm_hour, ltm->tm_min, ltm->tm_sec);

       }
       else {
          sprintf(buffer,"!!ltm was 0!!");
       }
      puts (buffer);

// "original" code - not working for me
       ltm = gmtime((time_t*)&ntpTime);

      puts ("Testing version 2");
      if (ltm != 0) {
      sprintf(buffer,"\"StartupUTC\":\"%04d-%02d-%02dT%02d:%02d:%02d\",", ltm->tm_year + 1900, ltm->tm_mon + 1, ltm->tm_mday, ltm->tm_hour, ltm->tm_min, ltm->tm_sec);

      }
       else {
          sprintf(buffer,"!!ltm was 0!!");
       }
      puts (buffer);
  return 0;
}

The output is

Testing version 1
"StartupUTC":"2024-02-13T15:49:24",
Testing version 2
!!ltm was 0!!

[EDIT] Just one other point, but I might be mistaken: Is NTP_GetCurrentTime() - g_secondsElapsed really the correct variable here? shouldn't it be NTP_GetCurrentTimeWithoutOffset() - g_secondsElapsed to get UTC time? [/EDIT]

MaxineMuster commented 8 months ago

Fixed with latest changes in https://github.com/openshwprojects/OpenBK7231T_App/pull/1150