deny control characters in cells and plane emissions #1840

doc/man/man3/notcurses_output.3.md

@@ -100,6 +100,9 @@ breaks. For more information, consult [Unicode® Standard Annex #29](https://uni
 Functions accepting a set of EGCs must consist of a series of well-formed EGCs,
 broken by cluster breaks, terminated by the appropriate NUL terminator.
 
+Control characters are rejected, except for a newline when the output plane
+is in scrolling mode. A newline outside of scrolling mode will be rejected.
+
 These functions output to the `ncplane`'s current cursor location. Aside from
 **ncplane_puttext()**, they *do not* move to the next line upon reaching the
 right extreme of the containing plane. If the entirety of the content cannot be

src/lib/internal.h

@@ -1474,6 +1474,25 @@ plane_blit_sixel(sprixel* spx, char* s, int bytes, int leny, int lenx,
   return 0;
 }
 
+// is it a control character? check C0 and C1, but don't count empty strings.
+static inline bool
+is_control_egc(const unsigned char* egc, int bytes){
+  if(bytes == 1){
+    if(iscntrl(*egc)){
+      return true;
+    }
+  }else if(bytes == 2){
+    // 0xc2 followed by 0x80--0x9f are controls. 0xc2 followed by <0x80 is
+    // simply invalid utf8.
+    if(egc[0] == 0xc2){
+      if(egc[1] < 0xa0){
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 // lowest level of cell+pool setup. if the EGC changes the output to RTL, it
 // must be suffixed with a LTR-forcing character by now. The four bits of
 // CELL_BLITTERSTACK_MASK ought already be initialized. If gcluster is four
@@ -1484,6 +1503,9 @@ pool_blit_direct(egcpool* pool, nccell* c, const char* gcluster, int bytes, int
   if(bytes < 0 || cols < 0){
     return -1;
   }
+  if(is_control_egc((const unsigned char*)gcluster, bytes)){
+    return -1;
+  }
   c->width = cols;
   if(bytes <= 4){
     c->gcluster = 0;

src/lib/notcurses.c

@@ -1546,13 +1546,23 @@ ncplane_put(ncplane* n, int y, int x, const char* egc, int cols,
     logerror("Can't write glyphs (%s) to sprixelated plane\n", egc);
     return -1;
   }
-  // FIXME reject any control or space characters here--should be iswgraph()
+  // reject any control character for output other than newline (and then only
+  // on a scrolling plane). this also rejects any 0-length EGC.
+  if(*egc == '\n'){
+    if(!n->scrolling){
+      logerror("Rejecting newline on non-scrolling plane\n");
+      return -1;
+    }
+  }else if(bytes == 0 || is_control_egc((const unsigned char*)egc, bytes)){
+    logerror("Rejecting %dB control character\n", bytes);
+    return -1;
+  }
   // check *before ncplane_cursor_move_yx()* whether we're past the end of the
   // line. if scrolling is enabled, move to the next line if so. if x or y are
   // specified, we must always try to print at exactly that location.
   if(x != -1){
     if(x + cols > n->lenx){
-      logerror("Target x %d + %d cols >= length %d\n", x, cols, n->lenx);
+      logerror("Target x %d + %d cols [%.*s] > length %d\n", x, cols, bytes, egc, n->lenx);
       ncplane_cursor_move_yx(n, y, x); // update cursor, though
       return -1;
     }
@@ -1566,9 +1576,6 @@ ncplane_put(ncplane* n, int y, int x, const char* egc, int cols,
   if(ncplane_cursor_move_yx(n, y, x)){
     return -1;
   }
-  // FIXME don't we need to check here for wide character on edge (though our
-  // docs currently claim that a wide char on edge is allowed...)?
-  // FIXME ought not be allowed when scrolling is disabled!
   if(*egc == '\n'){
     scroll_down(n);
     return 0;

src/tests/cell.cpp

@@ -328,7 +328,6 @@ TEST_CASE("Cell") {
     };
     auto np = ncplane_create(n_, &nopts);
     REQUIRE(nullptr != np);
-    CHECK(1 == ncplane_putc(np, &c));
     nccell_load_char(n_, &c, '*');
     // bottom has white foreground + HIGHCONTRAST, should remain white
     CHECK(0 == nccell_set_fg_rgb8(&c, 0xff, 0x0, 0xff));
@@ -356,11 +355,13 @@ TEST_CASE("Cell") {
     CHECK(0 == strcmp(nccell_extended_gcluster(n_, &c), "*"));
   }
 
+  // only space is allowed
   SUBCASE("CellLoadCharWhitespace") {
     nccell c = CELL_TRIVIAL_INITIALIZER;
-    CHECK(1 == nccell_load_char(n_, &c, '\f'));
-    CHECK(1 == nccell_load_char(n_, &c, '\n'));
-    CHECK(1 == nccell_load_char(n_, &c, '\t'));
+    CHECK(-1 == nccell_load_char(n_, &c, '\n'));
+    CHECK(-1 == nccell_load_char(n_, &c, '\f'));
+    CHECK(-1 == nccell_load_char(n_, &c, '\v'));
+    CHECK(-1 == nccell_load_char(n_, &c, '\t'));
     CHECK(1 == nccell_load_char(n_, &c, ' '));
   }
 

src/tests/plane.cpp

@@ -987,6 +987,20 @@ TEST_CASE("Plane") {
     ncplane_destroy(n1);
   }
 
+  // you ought not be able to output control characters
+  SUBCASE("DenyControlASCII") {
+    signed char c = 0;
+    c = -1;
+    do{
+      ++c;
+      if(!isprint(c)){
+        CHECK(0 > ncplane_putchar_yx(n_, 0, 0, c));
+      }else{
+        CHECK(1 == ncplane_putchar_yx(n_, 0, 0, c));
+      }
+    }while(c != 127);
+  }
+
   CHECK(0 == notcurses_stop(nc_));
 
 }