From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 32072 invoked from network); 24 Nov 2021 23:05:31 -0000 Received: from zero.zsh.org (2a02:898:31:0:48:4558:7a:7368) by inbox.vuxu.org with ESMTPUTF8; 24 Nov 2021 23:05:31 -0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=zsh.org; s=rsa-20210803; h=List-Archive:List-Owner:List-Post:List-Unsubscribe: List-Subscribe:List-Help:List-Id:Sender:Message-ID:Date:Content-ID: Content-Type:MIME-Version:Subject:To:References:From:In-reply-to:cc:Reply-To: Content-Transfer-Encoding:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=UNbc8LFWxtr2IUarmSj1fUkkLkXHLECS4frrcOJfP4M=; b=YjmgfLpd+9cmHcaJQNAXUppqkG t5j+F8nLtvUNhDJsOX+Am2htUY4Lojeo3+CgaDlzlycxBm5w5UflOt8z0N7AeKkmRBJgqUcuadS4W Ce+tX2e1IB0j9jRm05jamW+HuFPlK1QoBhV1Z6s2a4kte9FawTCBn8YLgPTxxFBvqJ8UFcs3V8jzx MYRyaRH/IhqvAV4/QfeUQgFd99rMkQMhaNKIySA5qxx3KiyFqHsNKqpcweXROgZ/7WQSIXMKDjbxI UX4ps5FCCM4aWcDTGngDSwRllZgl0n3hJOG66hnN9MeV5eLVxobPol9NPZExU5Fbk19sFR1S/frob gUQVu4dQ==; Received: from authenticated user by zero.zsh.org with local id 1mq1KT-000CyY-Rw; Wed, 24 Nov 2021 23:05:29 +0000 Received: from authenticated user by zero.zsh.org with esmtpsa (TLS1.3:TLS_AES_256_GCM_SHA384:256) id 1mq1Jv-000Cgs-P0; Wed, 24 Nov 2021 23:04:55 +0000 Received: from [192.168.178.21] (helo=hydra) by mail.kiddle.eu with esmtp(Exim 4.94.2) (envelope-from ) id 1mq1Ju-000CyB-HD; Thu, 25 Nov 2021 00:04:54 +0100 cc: zsh-workers@zsh.org In-reply-to: <87v911ymjw.fsf@vuxu.org> From: Oliver Kiddle References: <87v911ymjw.fsf@vuxu.org> To: Leah Neukirchen Subject: Re: [BUG] Trailing backslash in history file confuses entries MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <49857.1637795094.1@hydra> Date: Thu, 25 Nov 2021 00:04:54 +0100 Message-ID: <49858-1637795094.530308@-8CI.F0za.0QQo> X-Seq: 49601 Archived-At: X-Loop: zsh-workers@zsh.org Errors-To: zsh-workers-owner@zsh.org Precedence: list Precedence: bulk Sender: zsh-workers-request@zsh.org X-no-archive: yes List-Id: List-Help: List-Subscribe: List-Unsubscribe: List-Post: List-Owner: List-Archive: On 9 Nov, Leah Neukirchen wrote: > rhea% less foo\ # this saves a line with final \ to .zsh_history Thanks for narrowing this down with a clear example. You can also break this with print -s but this effect of pressing Ctrl-C on a PS2 prompt is interesting in that I had never really considered it before. The history file use of a trailing backslash to indicate continuation lines has caused problems before, e.g. in 32882 a space was added where lines end in an even number of backslashes. The patch below includes a test case that puts problematic strings in history, writes them and reads them back in. I'd be grateful if anyone else can think of further cases and history file contents that might cause issues. What this test case doesn't encapsulate is requirements in terms of the on-disk history file format. Are there any requirements here that I might have missed? It's clear that some care has gone into using something that it is close to being valid shell input but it isn't for multi-line commands. (ksh93 uses doubled null characters which you can't reinput and Bash reformats multi-line commands to be single-line commands.) Currently an extra space is added where the full line ends in an even number of backslashes. The approach taken in this patch is to add a single space to any line that ends with any number of backslashes followed by zero or more spaces. It then strips one space when reading history if the line ends with backslash followed by one or more spaces. A backslash followed directly by a newline now always indicates a line break within a command-line. I hope this preserves original lines without introducing new issues. And compatibility with old history files is only broken for fairly contrived cases or those that were already broken (such as that demonstrated by Leah). It has no problems with my history file. Oliver diff --git a/Src/hist.c b/Src/hist.c index 6ac581fda..af62cf931 100644 --- a/Src/hist.c +++ b/Src/hist.c @@ -2615,12 +2615,19 @@ readhistline(int start, char **bufp, int *bufsiz, FILE *in, int *readbytes) } } else { + int spc; buf[len - 1] = '\0'; if (len > 1 && buf[len - 2] == '\\') { buf[--len - 1] = '\n'; if (!feof(in)) return readhistline(len, bufp, bufsiz, in, readbytes); } + + spc = len - 2; + while (spc >= 0 && buf[spc] == ' ') + spc--; + if (spc != len - 2 && buf[spc] == '\\') + buf[--len - 1] = '\0'; } return len; } @@ -2984,7 +2991,7 @@ savehistfile(char *fn, int err, int writeflags) ret = 0; for (; he && he->histnum <= xcurhist; he = down_histent(he)) { - int count_backslashes = 0; + int end_backslashes = 0; if ((writeflags & HFILE_SKIPDUPS && he->node.flags & HIST_DUP) || (writeflags & HFILE_SKIPFOREIGN && he->node.flags & HIST_FOREIGN) @@ -3017,18 +3024,14 @@ savehistfile(char *fn, int err, int writeflags) if (*t == '\n') if ((ret = fputc('\\', out)) < 0) break; - if (*t == '\\') - count_backslashes++; - else - count_backslashes = 0; + end_backslashes = (*t == '\\' || (end_backslashes && *t == ' ')); if ((ret = fputc(*t, out)) < 0) break; } if (ret < 0) break; - if (count_backslashes && (count_backslashes % 2 == 0)) - if ((ret = fputc(' ', out)) < 0) - break; + if (end_backslashes) + ret = fputc(' ', out); if (ret < 0 || (ret = fputc('\n', out)) < 0) break; } diff --git a/Test/W01history.ztst b/Test/W01history.ztst index 0b2f60d1e..1d3f3cf6f 100644 --- a/Test/W01history.ztst +++ b/Test/W01history.ztst @@ -88,3 +88,25 @@ F:Check that a history bug introduced by workers/34160 is working again. 0:Modifier :P >/my/path/for/testing >/my/path/for/testing + + $ZTST_testdir/../Src/zsh -fgis <<<' + SAVEHIST=7 + print -rs "one\\" + print -rs "two\\\\" + print -rs "three\\\\\\" + print -rs "four\\\\\\\\" + print -rs "five\\\\\\\\\\" + print -s "while false\ndo\ntrue\\\\\n && break\ndone" + print -s "echo one\\\\\ntwo" + fc -W hist + fc -p -R hist + fc -l + rm hist' 2>/dev/null +0:Lines ending in backslash saved and restored to history +> 1 one\ +> 2 two\\ +> 3 three\\\ +> 4 four\\\\ +> 5 five\\\\\ +> 6 while false\ndo\ntrue\\n && break\ndone +> 7 echo one\\ntwo