From 2e137c0a7e747a781ec311c6aad656873072b96c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 16 Dec 2020 14:30:49 +0100 Subject: [PATCH] =?UTF-8?q?vt:=20don=E2=80=99t=20ignore=20extra=20private/?= =?UTF-8?q?intermediate=20characters?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Take ‘\E(#0’ for example - this is *not* the same as ‘\E(0’. Up until now, foot has however treated them as the same escape, because the handler for ‘\E(0’ didn’t verify there weren’t any _other_ private characters present. Fix this by turning the ‘private’ array into a single 4-byte integer. This allows us to match *all* privates with a single comparison. Private characters are added to the LSB first, and MSB last. This means we can check for single privates in pretty much the same way as before: switch (term->vt.private) { case ‘?’: ... break; } Checking for two (or more) is much uglier, but foot only supports a *single* escape with two privates, and no escapes with three or more: switch (term->vt.private) { case 0x243f: /* ‘?$’ */ ... break; } The ‘clear’ action remains simple (and fast), with a single write operation. Collecting privates is potentially _slightly_ more complex than before; we now need mask and compare, instead of simply comparing, when checking how many privates we already have. We _could_ add a counter, which would make collecting privates easier, but this would add an additional write to the ‘clean’ action which is really bad since it’s in the hot path. --- CHANGELOG.md | 2 ++ csi.c | 88 ++++++++++++++++++++++++++++------------------------ dcs.c | 6 ++-- terminal.h | 2 +- vt.c | 40 ++++++++++++++++-------- 5 files changed, 80 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e0de16c..5051283d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -139,6 +139,8 @@ means foot can be PGO:d in e.g. sandboxed build scripts. See * Frames occasionally being rendered while application synchronized updates is in effect. * Handling of failures to parse the font specification string. +* Extra private/intermediate characters in escape sequences not being + ignored. ### Security diff --git a/csi.c b/csi.c index 5ecb018d..14d41c12 100644 --- a/csi.c +++ b/csi.c @@ -57,10 +57,11 @@ csi_as_string(struct terminal *term, uint8_t final, int idx) i == term->vt.params.idx - 1 ? "" : ";"); } - for (size_t i = 0; i < sizeof(term->vt.private) / sizeof(term->vt.private[0]); i++) { - if (term->vt.private[i] == 0) + for (size_t i = 0; i < sizeof(term->vt.private); i++) { + char value = (term->vt.private >> (i * 8)) & 0xff; + if (value == 0) break; - c += snprintf(&msg[c], sizeof(msg) - c, "%c", term->vt.private[i]); + c += snprintf(&msg[c], sizeof(msg) - c, "%c", value); } snprintf(&msg[c], sizeof(msg) - c, "%c (%u parameters)", @@ -673,9 +674,9 @@ xtrestore(struct terminal *term, unsigned param) void csi_dispatch(struct terminal *term, uint8_t final) { - LOG_DBG("%s", csi_as_string(term, final, -1)); + LOG_DBG("%s (%08x)", csi_as_string(term, final, -1), term->vt.private); - switch (term->vt.private[0]) { + switch (term->vt.private) { case 0: { switch (final) { case 'b': @@ -1361,7 +1362,7 @@ csi_dispatch(struct terminal *term, uint8_t final) break; } - break; /* private == 0 */ + break; /* private[0] == 0 */ } case '?': { @@ -1378,36 +1379,6 @@ csi_dispatch(struct terminal *term, uint8_t final) decrst(term, term->vt.params.v[i].value); break; - case 'p': { - if (term->vt.private[1] != '$') { - UNHANDLED(); - break; - } - - unsigned param = vt_param_get(term, 0, 0); - - /* - * Request DEC private mode (DECRQM) - * Reply: - * 0 - not recognized - * 1 - set - * 2 - reset - * 3 - permanently set - * 4 - permantently reset - */ - bool enabled; - unsigned value; - if (decrqm(term, param, &enabled)) - value = enabled ? 1 : 2; - else - value = 0; - - char reply[32]; - snprintf(reply, sizeof(reply), "\033[?%u;%u$y", param, value); - term_to_slave(term, reply, strlen(reply)); - break; - } - case 's': for (size_t i = 0; i < term->vt.params.idx; i++) xtsave(term, term->vt.params.v[i].value); @@ -1456,7 +1427,7 @@ csi_dispatch(struct terminal *term, uint8_t final) break; } - break; /* private == '?' */ + break; /* private[0] == '?' */ } case '>': { @@ -1542,7 +1513,7 @@ csi_dispatch(struct terminal *term, uint8_t final) break; } - break; /* private == '>' */ + break; /* private[0] == '>' */ } case ' ': { @@ -1587,7 +1558,7 @@ csi_dispatch(struct terminal *term, uint8_t final) UNHANDLED(); break; } - break; /* private == ' ' */ + break; /* private[0] == ' ' */ } case '!': { @@ -1600,7 +1571,7 @@ csi_dispatch(struct terminal *term, uint8_t final) UNHANDLED(); break; } - break; /* private == '!' */ + break; /* private[0] == '!' */ } case '=': { @@ -1629,9 +1600,44 @@ csi_dispatch(struct terminal *term, uint8_t final) UNHANDLED(); break; } - break; /* private == '=' */ + break; /* private[0] == '=' */ } + case 0x243f: /* ?$ */ + switch (final) { + case 'p': { + unsigned param = vt_param_get(term, 0, 0); + + /* + * Request DEC private mode (DECRQM) + * Reply: + * 0 - not recognized + * 1 - set + * 2 - reset + * 3 - permanently set + * 4 - permantently reset + */ + bool enabled; + unsigned value; + if (decrqm(term, param, &enabled)) + value = enabled ? 1 : 2; + else + value = 0; + + char reply[32]; + snprintf(reply, sizeof(reply), "\033[?%u;%u$y", param, value); + term_to_slave(term, reply, strlen(reply)); + break; + + } + + default: + UNHANDLED(); + break; + } + + break; /* private[0] == ‘?’ && private[1] == ‘$’ */ + default: UNHANDLED(); break; diff --git a/dcs.c b/dcs.c index f9e3c10c..7d0dd6cc 100644 --- a/dcs.c +++ b/dcs.c @@ -31,15 +31,15 @@ esu(struct terminal *term) void dcs_hook(struct terminal *term, uint8_t final) { - LOG_DBG("hook: %c (intermediate(s): %.2s, param=%d)", final, term->vt.private, - vt_param_get(term, 0, 0)); + LOG_DBG("hook: %c (intermediate(s): %.2s, param=%d)", final, + (const char *)&term->vt.private, vt_param_get(term, 0, 0)); assert(term->vt.dcs.data == NULL); assert(term->vt.dcs.size == 0); assert(term->vt.dcs.put_handler == NULL); assert(term->vt.dcs.unhook_handler == NULL); - switch (term->vt.private[0]) { + switch (term->vt.private) { case 0: switch (final) { case 'q': diff --git a/terminal.h b/terminal.h index 8708661f..e13243a2 100644 --- a/terminal.h +++ b/terminal.h @@ -134,7 +134,7 @@ struct vt { struct vt_param v[16]; uint8_t idx; } params; - char private[2]; + uint32_t private; /* LSB=priv0, MSB=priv3 */ struct { uint8_t *data; size_t size; diff --git a/vt.c b/vt.c index ed26da73..37b80eec 100644 --- a/vt.c +++ b/vt.c @@ -82,10 +82,11 @@ esc_as_string(struct terminal *term, uint8_t final) static char msg[1024]; int c = snprintf(msg, sizeof(msg), "\\E"); - for (size_t i = 0; i < sizeof(term->vt.private) / sizeof(term->vt.private[0]); i++) { - if (term->vt.private[i] == 0) + for (size_t i = 0; i < sizeof(term->vt.private); i++) { + char value = (term->vt.private >> (i * 8)) & 0xff; + if (value == 0) break; - c += snprintf(&msg[c], sizeof(msg) - c, "%c", term->vt.private[i]); + c += snprintf(&msg[c], sizeof(msg) - c, "%c", value); } assert(term->vt.params.idx == 0); @@ -105,8 +106,7 @@ static void action_clear(struct terminal *term) { term->vt.params.idx = 0; - term->vt.private[0] = 0; - term->vt.private[1] = 0; + term->vt.private = 0; } static void @@ -335,12 +335,26 @@ static void action_collect(struct terminal *term, uint8_t c) { LOG_DBG("collect: %c", c); - if (term->vt.private[0] == 0) - term->vt.private[0] = c; - else if (term->vt.private[1] == 0) - term->vt.private[1] = c; + + /* + * Having more than one private is *very* rare. Foot ony supports + * a *single* escape with two privates, and none with three or + * more. + * + * As such, we optimize *reading* the private(s), and *resetting* + * them (in action_clear()). Writing is ok if it’s a bit slow. + */ + + if ((term->vt.private & 0xff) == 0) + term->vt.private = c; + else if (((term->vt.private >> 8) & 0xff) == 0) + term->vt.private |= c << 8; + else if (((term->vt.private >> 16) & 0xff) == 0) + term->vt.private |= c << 16; + else if (((term->vt.private >> 24) & 0xff) == 0) + term->vt.private |= c << 24; else - LOG_WARN("only two private/intermediate characters supported"); + LOG_WARN("only four private/intermediate characters supported"); } static void @@ -348,7 +362,7 @@ action_esc_dispatch(struct terminal *term, uint8_t final) { LOG_DBG("ESC: %s", esc_as_string(term, final)); - switch (term->vt.private[0]) { + switch (term->vt.private) { case 0: switch (final) { case '7': @@ -425,7 +439,7 @@ action_esc_dispatch(struct terminal *term, uint8_t final) case '+': switch (final) { case '0': { - char priv = term->vt.private[0]; + char priv = term->vt.private; ssize_t idx = priv == '(' ? 0 : ')' ? 1 : @@ -437,7 +451,7 @@ action_esc_dispatch(struct terminal *term, uint8_t final) } case 'B': { - char priv = term->vt.private[0]; + char priv = term->vt.private; ssize_t idx = priv == '(' ? 0 : ')' ? 1 :