[1.1.x] Buffer overflow and scroll fix, UTF8 cleanup (#10851)

Marlin/G26_Mesh_Validation_Tool.cpp

@@ -134,9 +134,6 @@
   // External references
 
   extern Planner planner;
-  #if ENABLED(ULTRA_LCD)
-    extern char lcd_status_message[];
-  #endif
 
   // Private functions
 
@@ -274,8 +271,6 @@
 
         wait_for_release();
 
-        strcpy_P(lcd_status_message, PSTR("Done Priming")); // Hack to get the message up. May be obsolete.
-
         lcd_setstatusPGM(PSTR("Done Priming"), 99);
         lcd_quick_feedback(true);
         lcd_external_control = false;

Marlin/Marlin_main.cpp

@@ -5894,15 +5894,12 @@ void home_all_axes() { gcode_G28(true); }
     }
 
     // Report settings
-
     const char *checkingac = PSTR("Checking... AC");
     serialprintPGM(checkingac);
     if (verbose_level == 0) SERIAL_PROTOCOLPGM(" (DRY-RUN)");
     if (set_up) SERIAL_PROTOCOLPGM("  (SET-UP)");
     SERIAL_EOL();
-    char mess[11];
-    strcpy_P(mess, checkingac);
-    lcd_setstatus(mess);
+    lcd_setstatusPGM(checkingac);
 
     print_calibration_settings(_endstop_results, _angle_results);
 

Marlin/status_screen_DOGM.h

@@ -124,36 +124,77 @@ FORCE_INLINE void _draw_axis_value(const AxisEnum axis, const char *value, const
 inline void lcd_implementation_status_message(const bool blink) {
   #if ENABLED(STATUS_MESSAGE_SCROLLING)
     static bool last_blink = false;
-    const uint8_t slen = lcd_strlen(lcd_status_message);
-    const char *stat = lcd_status_message + status_scroll_pos;
-    if (slen <= LCD_WIDTH)
-      lcd_print_utf(stat);                                      // The string isn't scrolling
+    
+    // Get the UTF8 character count of the string
+    uint8_t slen = lcd_strlen(lcd_status_message);
+
+    // If the string fits into the LCD, just print it and do not scroll it
+    if (slen <= LCD_WIDTH) {
+
+      // The string isn't scrolling and may not fill the screen
+      lcd_print_utf(lcd_status_message);
+
+      // Fill the rest with spaces
+      while (slen < LCD_WIDTH) {
+        u8g.print(' ');
+        ++slen;
+      }
+    }
     else {
-      if (status_scroll_pos <= slen - LCD_WIDTH)
-        lcd_print_utf(stat);                                    // The string fills the screen
+      // String is larger than the available space in screen.
+
+      // Get a pointer to the next valid UTF8 character
+      const char *stat = lcd_status_message + status_scroll_offset;
+
+      // Get the string remaining length
+      const uint8_t rlen = lcd_strlen(stat);
+
+      // If we have enough characters to display
+      if (rlen >= LCD_WIDTH) {
+        // The remaining string fills the screen - Print it
+        lcd_print_utf(stat, LCD_WIDTH);
+      }
       else {
-        uint8_t chars = LCD_WIDTH;
-        if (status_scroll_pos < slen) {                         // First string still visible
-          lcd_print_utf(stat);                                  // The string leaves space
-          chars -= slen - status_scroll_pos;                    // Amount of space left
-        }
-        u8g.print('.');                                         // Always at 1+ spaces left, draw a dot
-        if (--chars) {
-          if (status_scroll_pos < slen + 1)                     // Draw a second dot if there's space
-            --chars, u8g.print('.');
-          if (chars) lcd_print_utf(lcd_status_message, chars);  // Print a second copy of the message
+        // The remaining string does not completely fill the screen
+        lcd_print_utf(stat, LCD_WIDTH);         // The string leaves space
+        uint8_t chars = LCD_WIDTH - rlen;       // Amount of space left in characters
+
+        u8g.print('.');                         // Always at 1+ spaces left, draw a dot
+        if (--chars) {                          // Draw a second dot if there's space
+          u8g.print('.');
+          if (--chars) {
+            // Print a second copy of the message
+            lcd_print_utf(lcd_status_message, LCD_WIDTH - (rlen+2)); 
+          }
         }
       }
-      if (last_blink != blink) {
-        last_blink = blink;
-        // Skip any non-printing bytes
-        if (status_scroll_pos < slen) while (!PRINTABLE(lcd_status_message[status_scroll_pos])) status_scroll_pos++;
-        if (++status_scroll_pos >= slen + 2) status_scroll_pos = 0;
+       if (last_blink != blink) {
+         last_blink = blink;
+
+        // Adjust by complete UTF8 characters
+        if (status_scroll_offset < slen) {
+          status_scroll_offset++;
+          while (!START_OF_UTF8_CHAR(lcd_status_message[status_scroll_offset]))
+            status_scroll_offset++;
+        }
+        else
+          status_scroll_offset = 0;
       }
     }
   #else
     UNUSED(blink);
-    lcd_print_utf(lcd_status_message);
+
+    // Get the UTF8 character count of the string
+    uint8_t slen = lcd_strlen(lcd_status_message);
+
+    // Just print the string to the LCD
+    lcd_print_utf(lcd_status_message, LCD_WIDTH);
+
+    // Fill the rest with spaces if there are missing spaces
+    while (slen < LCD_WIDTH) {
+      u8g.print(' ');
+      ++slen;
+    }
   #endif
 }
 

Marlin/status_screen_lite_ST7920.h

@@ -615,36 +615,71 @@ void ST7920_Lite_Status_Screen::draw_feedrate_percentage(const uint8_t percentag
 void ST7920_Lite_Status_Screen::draw_status_message(const char *str) {
   set_ddram_address(DDRAM_LINE_4);
   begin_data();
+  const uint8_t lcd_len = 16;
   #if ENABLED(STATUS_MESSAGE_SCROLLING)
-    const uint8_t lcd_len = 16;
-    const uint8_t padding = 2;
-    uint8_t str_len = strlen(str);
+ 
+    uint8_t slen = lcd_strlen(str);
 
-    // Trim whitespace at the end of the str, as for some reason
-    // messages like "Card Inserted" are padded with many spaces
-    while (str_len && str[str_len - 1] == ' ') str_len--;
+    // If the string fits into the LCD, just print it and do not scroll it
+    if (slen <= lcd_len) {
 
-    if (str_len <= lcd_len) {
-      // It all fits on the LCD without scrolling
+      // The string isn't scrolling and may not fill the screen
       write_str(str);
+
+      // Fill the rest with spaces
+      while (slen < lcd_len) {
+        write_byte(' ');
+        ++slen;
+      }
     }
     else {
-      // Print the message repeatedly until covering the LCD
-      uint8_t c = status_scroll_pos;
-      for (uint8_t n = 0; n < lcd_len; n++) {
-        write_byte(c < str_len ? str[c] : ' ');
-        c++;
-        c %= str_len + padding; // Wrap around
+      // String is larger than the available space in screen.
+
+      // Get a pointer to the next valid UTF8 character
+      const char *stat = str + status_scroll_offset;
+
+      // Get the string remaining length
+      const uint8_t rlen = lcd_strlen(stat);
+
+      // If we have enough characters to display
+      if (rlen >= lcd_len) {
+        // The remaining string fills the screen - Print it
+        write_str(stat, lcd_len);
+      }
+      else {
+        // The remaining string does not completely fill the screen
+        write_str(stat);                        // The string leaves space
+        uint8_t chars = lcd_len - rlen;         // Amount of space left in characters
+
+        write_byte('.');                        // Always at 1+ spaces left, draw a dot
+        if (--chars) {                          // Draw a second dot if there's space
+          write_byte('.');
+          if (--chars)
+            write_str(str, chars);              // Print a second copy of the message
+        }
       }
 
-      // Scroll the message
-      if (status_scroll_pos == str_len + padding)
-        status_scroll_pos = 0;
+      // Adjust by complete UTF8 characters
+      if (status_scroll_offset < slen) {
+        status_scroll_offset++;
+        while (!START_OF_UTF8_CHAR(str[status_scroll_offset]))
+          status_scroll_offset++;
+      }
       else
-        status_scroll_pos++;
+        status_scroll_offset = 0;
     }
   #else
-    write_str(str, 16);
+    // Get the UTF8 character count of the string
+    uint8_t slen = lcd_strlen(str);
+
+    // Just print the string to the LCD
+    write_str(str, lcd_len);
+
+    // Fill the rest with spaces if there are missing spaces
+    while (slen < lcd_len) {
+      write_byte(' ');
+      ++slen;
+    }
   #endif
 }
 
@@ -792,7 +827,7 @@ void ST7920_Lite_Status_Screen::update_status_or_position(bool forceUpdate) {
    */
   if (forceUpdate || status_changed()) {
     #if ENABLED(STATUS_MESSAGE_SCROLLING)
-      status_scroll_pos = 0;
+      status_scroll_offset = 0;
     #endif
     #if STATUS_EXPIRE_SECONDS
       countdown = lcd_status_message[0] ? STATUS_EXPIRE_SECONDS : 0;

Marlin/ubl.h

@@ -61,7 +61,6 @@ extern uint8_t ubl_cnt;
 ///////////////////////////////////////////////////////////////////////////////////////////////////////
 
 #if ENABLED(ULTRA_LCD)
-  extern char lcd_status_message[];
   void lcd_quick_feedback(const bool clear_buttons);
 #endif
 

Marlin/ultralcd.cpp

@@ -71,7 +71,7 @@
   #else
     #define MAX_MESSAGE_LENGTH CHARSIZE * 2 * (LCD_WIDTH)
   #endif
-  uint8_t status_scroll_pos = 0;
+  uint8_t status_scroll_offset = 0;
 #else
   #define MAX_MESSAGE_LENGTH CHARSIZE * (LCD_WIDTH)
 #endif
@@ -5030,7 +5030,7 @@ void lcd_init() {
 int16_t lcd_strlen(const char* s) {
   int16_t i = 0, j = 0;
   while (s[i]) {
-    if (PRINTABLE(s[i])) j++;
+    if (START_OF_UTF8_CHAR(s[i])) j++;
     i++;
   }
   return j;
@@ -5039,7 +5039,7 @@ int16_t lcd_strlen(const char* s) {
 int16_t lcd_strlen_P(const char* s) {
   int16_t j = 0;
   while (pgm_read_byte(s)) {
-    if (PRINTABLE(pgm_read_byte(s))) j++;
+    if (START_OF_UTF8_CHAR(pgm_read_byte(s))) j++;
     s++;
   }
   return j;
@@ -5367,30 +5367,8 @@ void lcd_update() {
   } // ELAPSED(ms, next_lcd_update_ms)
 }
 
-inline void pad_message_string() {
-  uint8_t i = 0, j = 0;
-  char c;
-  lcd_status_message[MAX_MESSAGE_LENGTH] = '\0';
-  while ((c = lcd_status_message[i]) && j < LCD_WIDTH) {
-    if (PRINTABLE(c)) j++;
-    i++;
-  }
-  if (true
-    #if ENABLED(STATUS_MESSAGE_SCROLLING)
-      && j < LCD_WIDTH
-    #endif
-  ) {
-    // pad with spaces to fill up the line
-    while (j++ < LCD_WIDTH) lcd_status_message[i++] = ' ';
-    // chop off at the edge
-    lcd_status_message[i] = '\0';
-  }
-}
-
 void lcd_finishstatus(const bool persist=false) {
 
-  pad_message_string();
-
   #if !(ENABLED(LCD_PROGRESS_BAR) && (PROGRESS_MSG_EXPIRE > 0))
     UNUSED(persist);
   #endif
@@ -5408,7 +5386,7 @@ void lcd_finishstatus(const bool persist=false) {
   #endif
 
   #if ENABLED(STATUS_MESSAGE_SCROLLING)
-    status_scroll_pos = 0;
+    status_scroll_offset = 0;
   #endif
 }
 
@@ -5420,7 +5398,26 @@ bool lcd_hasstatus() { return (lcd_status_message[0] != '\0'); }
 
 void lcd_setstatus(const char * const message, const bool persist) {
   if (lcd_status_message_level > 0) return;
-  strncpy(lcd_status_message, message, MAX_MESSAGE_LENGTH);
+
+  // Here we have a problem. The message is encoded in UTF8, so
+  // arbitrarily cutting it will be a problem. We MUST be sure
+  // that there is no cutting in the middle of a multibyte character!
+
+  // Get a pointer to the null terminator
+  const char* pend = message + strlen(message);
+
+  //  If length of supplied UTF8 string is greater than
+  // our buffer size, start cutting whole UTF8 chars
+  while ((pend - message) > MAX_MESSAGE_LENGTH) {
+    --pend;
+    while (!START_OF_UTF8_CHAR(*pend)) --pend;
+  };
+
+  // At this point, we have the proper cut point. Use it
+  uint8_t maxLen = pend - message;
+  strncpy(lcd_status_message, message, maxLen);
+  lcd_status_message[maxLen] = '\0';
+
   lcd_finishstatus(persist);
 }
 
@@ -5428,7 +5425,26 @@ void lcd_setstatusPGM(const char * const message, int8_t level) {
   if (level < 0) level = lcd_status_message_level = 0;
   if (level < lcd_status_message_level) return;
   lcd_status_message_level = level;
-  strncpy_P(lcd_status_message, message, MAX_MESSAGE_LENGTH);
+
+  // Here we have a problem. The message is encoded in UTF8, so
+  // arbitrarily cutting it will be a problem. We MUST be sure
+  // that there is no cutting in the middle of a multibyte character!
+
+  // Get a pointer to the null terminator
+  const char* pend = message + strlen_P(message);
+
+  //  If length of supplied UTF8 string is greater than
+  // our buffer size, start cutting whole UTF8 chars
+  while ((pend - message) > MAX_MESSAGE_LENGTH) {
+    --pend;
+    while (!START_OF_UTF8_CHAR(pgm_read_byte(pend))) --pend;
+  };
+
+  // At this point, we have the proper cut point. Use it
+  uint8_t maxLen = pend - message;
+  strncpy_P(lcd_status_message, message, maxLen);
+  lcd_status_message[maxLen] = '\0';
+
   lcd_finishstatus(level > 0);
 }
 

Marlin/ultralcd_impl_HD44780.h

@@ -491,13 +491,42 @@ void lcd_printPGM_utf(const char *str, uint8_t n=LCD_WIDTH) {
 
   // Scroll the PSTR 'text' in a 'len' wide field for 'time' milliseconds at position col,line
   void lcd_scroll(const int16_t col, const int16_t line, const char* const text, const int16_t len, const int16_t time) {
-    char tmp[LCD_WIDTH + 1] = {0};
-    const int16_t n = MAX(lcd_strlen_P(text) - len, 0);
-    for (int16_t i = 0; i <= n; i++) {
-      strncpy_P(tmp, text + i, MIN(len, LCD_WIDTH));
+    uint8_t slen = lcd_strlen_P(text);
+    if (slen < len) {
+      // Fits into,
       lcd.setCursor(col, line);
-      lcd_print(tmp);
-      delay(time / MAX(n, 1));
+      lcd_printPGM_utf(text, len);
+      while (slen < len) {
+        lcd.write(' ');
+        ++slen;
+      }
+      safe_delay(time);
+    }
+    else {
+      const char* p = text;
+      int dly = time / MAX(slen, 1);
+      for (uint8_t i = 0; i <= slen; i++) {
+
+        // Go to the correct place
+        lcd.setCursor(col, line);
+
+        // Print the text
+        lcd_printPGM_utf(p, len);
+
+        // Fill with spaces
+        uint8_t ix = slen - i;
+        while (ix < len) {
+          lcd.write(' ');
+          ++ix;
+        }
+
+        // Delay
+        safe_delay(dly);
+
+        // Advance to the next UTF8 valid position
+        p++;
+        while (!START_OF_UTF8_CHAR(pgm_read_byte(p))) p++;
+      }
     }
   }
 
@@ -895,38 +924,82 @@ static void lcd_implementation_status_screen() {
 
   #if ENABLED(STATUS_MESSAGE_SCROLLING)
     static bool last_blink = false;
-    const uint8_t slen = lcd_strlen(lcd_status_message);
-    const char *stat = lcd_status_message + status_scroll_pos;
-    if (slen <= LCD_WIDTH)
-      lcd_print_utf(stat);                                      // The string isn't scrolling
+
+    // Get the UTF8 character count of the string
+    uint8_t slen = lcd_strlen(lcd_status_message);
+
+    // If the string fits into the LCD, just print it and do not scroll it
+    if (slen <= LCD_WIDTH) {
+
+      // The string isn't scrolling and may not fill the screen
+      lcd_print_utf(lcd_status_message);
+
+      // Fill the rest with spaces
+      while (slen < LCD_WIDTH) {
+        lcd.write(' ');
+        ++slen;
+      }
+    }
     else {
-      if (status_scroll_pos <= slen - LCD_WIDTH)
-        lcd_print_utf(stat);                                    // The string fills the screen
+      // String is larger than the available space in screen.
+
+      // Get a pointer to the next valid UTF8 character
+      const char *stat = lcd_status_message + status_scroll_offset;
+
+      // Get the string remaining length
+      const uint8_t rlen = lcd_strlen(stat);
+
+      // If we have enough characters to display
+      if (rlen >= LCD_WIDTH) {
+        // The remaining string fills the screen - Print it
+        lcd_print_utf(stat, LCD_WIDTH);
+      }
       else {
-        uint8_t chars = LCD_WIDTH;
-        if (status_scroll_pos < slen) {                         // First string still visible
-          lcd_print_utf(stat);                                  // The string leaves space
-          chars -= slen - status_scroll_pos;                    // Amount of space left
-        }
-        lcd.write('.');                                         // Always at 1+ spaces left, draw a dot
-        if (--chars) {
-          if (status_scroll_pos < slen + 1)                     // Draw a second dot if there's space
-            --chars, lcd.write('.');
-          if (chars) lcd_print_utf(lcd_status_message, chars);  // Print a second copy of the message
+
+        // The remaining string does not completely fill the screen
+        lcd_print_utf(stat, LCD_WIDTH);               // The string leaves space
+        uint8_t chars = LCD_WIDTH - rlen;             // Amount of space left in characters
+
+        lcd.write('.');                               // Always at 1+ spaces left, draw a dot
+        if (--chars) {                                // Draw a second dot if there's space
+          lcd.write('.');
+          if (--chars)
+            lcd_print_utf(lcd_status_message, chars); // Print a second copy of the message
         }
       }
       if (last_blink != blink) {
         last_blink = blink;
-        // Skip any non-printing bytes
-        if (status_scroll_pos < slen) while (!PRINTABLE(lcd_status_message[status_scroll_pos])) status_scroll_pos++;
-        if (++status_scroll_pos >= slen + 2) status_scroll_pos = 0;
+
+        // Adjust by complete UTF8 characters
+        if (status_scroll_offset < slen) {
+          status_scroll_offset++;
+          while (!START_OF_UTF8_CHAR(lcd_status_message[status_scroll_offset]))
+            status_scroll_offset++;
+        }
+        else
+          status_scroll_offset = 0;
       }
     }
   #else
-    lcd_print_utf(lcd_status_message);
+    UNUSED(blink);
+
+    // Get the UTF8 character count of the string
+    uint8_t slen = lcd_strlen(lcd_status_message);
+
+    // Just print the string to the LCD
+    lcd_print_utf(lcd_status_message, LCD_WIDTH);
+
+    // Fill the rest with spaces if there are missing spaces
+    while (slen < LCD_WIDTH) {
+      lcd.write(' ');
+      ++slen;
+    }
   #endif
+
 }
 
+
+
 #if ENABLED(ULTIPANEL)
 
   #if ENABLED(ADVANCED_PAUSE_FEATURE)

Marlin/utf_mapper.h

@@ -144,7 +144,7 @@
   #endif // DISPLAY_CHARSET_HD44780
 #endif // SIMULATE_ROMFONT
 
-#define PRINTABLE(C) (((C) & 0xC0u) != 0x80u)
+#define START_OF_UTF8_CHAR(C) (((C) & 0xC0u) != 0x80u)
 
 #if ENABLED(MAPPER_C2C3)