I have been looking into PatchInfo parsing. It seems the parsing can be
easily broken leading to corrupt repos.
For instance, I just discovered that the patch name must not end with
']' or else unrecording the patch will hang in an endless loop. There
are no checks and you can create such a patch with darcs. (Also tested
with 2.10 so this is not a recent regression.)
Even worse things happen if you pass a non-ASCII date e.g. via --pipe.
This is quite horrible. The constraints required by the current
PatchInfo parser are nowhere documented and nowhere checked before
creating PatchInfos. I have a patch for some of this badness since a few
days but that doesn't cover the example above yet.
The example I gave is bogus, I tested it with a repo that was already
broken. Sorry. The point about validation stands, though, there are
certain restrictions to be observed, mainly for author and date.
Here is a summary of what the PatchInfo items may contain in order for
readPatchInfo to be able to parse back what showPatchinfo writes to disk.
date: any ASCII(!) character except '\n' or ']'
name,log: anything but '\n'
author: anything but '*'
The ASCII restriction for date is due to the use of BC.pack/unpack. The
other restrictions are necessary because the parser uses these
characters as delimiters of the fields in question. For the log the
restriction applies to each line of the log, of course.
Violating any of these invariants when constructing a PatchInfo can lead
to unparsable PatchInfos written to disk which is absolutely fatal for
users.
resolved by
patch 403698ffdc7b41b9840ccad632915f82a848b644
Author: Ben Franksen <ben.franksen@online.de>
Date: Mon Mar 12 22:42:53 CET 2018
* add validation for PatchInfo items, add unit test for
parse/unparse
History
Date
User
Action
Args
2018-03-17 17:03:11
bfrk
create
2018-03-17 17:22:36
bfrk
set
messages:
+ msg19978 title: PatchInfo parsing is broken / need validation -> PatchInfo parsing is broken / needs validation