From ccfc3d7bc78b5543bcb75def1511f47cf8e13b1e Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Thu, 13 Jun 2019 16:31:35 -0700 Subject: [PATCH] Validate commands in multiparser It was reported in https://github.com/sbt/sbt/issues/4808 that compared to 1.2.8, sbt 1.3.0-RC2 will truncate the command args of an input task that contains semicolons. This is actually intentional, but not completely robust. For sbt >= 1.3.0, we are making ';' syntactically meaningful. This means that it always represents a command separator _unless_ it is inside of a quoted string. To enforce this, the multi parser will effectively split the input on ';', it will then validate that each command that it extracted is valid. If not, it throws an exception. If the input is not a multi command, then parsing fails with a normal failure. I removed the multi command from the state's defined commands and reworked State.combinedParser to explicitly first try multi parsing and fall back to the regular combined parser if it is a regular command. If the multi parser throws an uncaught exception, parsing fails even if the regular parser could have successfully parsed the command. The reason is so that we do not ever allow the user to evaluate, say 'run a;b'. Otherwise the behavior would be inconsitent when the user runs 'compile; run a;b' --- .../src/main/scala/sbt/BasicCommands.scala | 21 ++++++++++++++----- main-command/src/main/scala/sbt/Command.scala | 3 +-- main-command/src/main/scala/sbt/State.scala | 3 ++- main/src/main/scala/sbt/Main.scala | 1 - .../sbt-test/actions/multi-command/build.sbt | 2 ++ .../actions/multi-command/project/Build.scala | 8 +++++++ sbt/src/sbt-test/actions/multi-command/test | 7 +++++++ 7 files changed, 36 insertions(+), 9 deletions(-) diff --git a/main-command/src/main/scala/sbt/BasicCommands.scala b/main-command/src/main/scala/sbt/BasicCommands.scala index daea8ed99..841244227 100644 --- a/main-command/src/main/scala/sbt/BasicCommands.scala +++ b/main-command/src/main/scala/sbt/BasicCommands.scala @@ -161,11 +161,22 @@ object BasicCommands { ((nonSemi & nonQuote).map(_.toString) | StringEscapable.map(c => s""""$c"""")).+, hide = const(true) ) - def commandParser = state.map(s => (s.combinedParser & cmdPart) | cmdPart).getOrElse(cmdPart) - val part = semi.flatMap(_ => matched(commandParser) <~ token(OptSpace)).map(_.trim) - (cmdPart.? ~ part.+ <~ semi.?).map { - case (Some(h), t) => h.mkString.trim +: t.toList - case (_, t) => t.toList + lazy val combinedParser = + state.map(s => (s.nonMultiParsers & cmdPart) | cmdPart).getOrElse(cmdPart) + val part = semi.flatMap(_ => matched(combinedParser) <~ token(OptSpace)) + (matched(cmdPart).? ~ part.+ <~ semi.?).map { + case (h, t) => + val commands = (h ++ t).toList.map(_.trim) + commands.collect { case c if Parser.parse(c, combinedParser).isLeft => c } match { + case Nil => commands + case invalid => + /* + * This is to prevent the user from running something like 'run a;b'. Instead, they + * must do 'run "a;b"' if they wish to have semicolongs in the task input. + */ + val msg = s"Couldn't parse commands: ${invalid.mkString("'", "', '", "'")}" + throw new IllegalArgumentException(msg) + } } } diff --git a/main-command/src/main/scala/sbt/Command.scala b/main-command/src/main/scala/sbt/Command.scala index ea616cb6e..89ba7a375 100644 --- a/main-command/src/main/scala/sbt/Command.scala +++ b/main-command/src/main/scala/sbt/Command.scala @@ -178,8 +178,7 @@ object Command { ) def process(command: String, state: State): State = { - val parser = combine(state.definedCommands) - parse(command, parser(state)) match { + parse(command, state.combinedParser) match { case Right(s) => s() // apply command. command side effects happen here case Left(errMsg) => state.log error errMsg diff --git a/main-command/src/main/scala/sbt/State.scala b/main-command/src/main/scala/sbt/State.scala index 62a0920a1..af81a96de 100644 --- a/main-command/src/main/scala/sbt/State.scala +++ b/main-command/src/main/scala/sbt/State.scala @@ -41,7 +41,8 @@ final case class State( currentCommand: Option[Exec], next: State.Next ) extends Identity { - lazy val combinedParser = Command.combine(definedCommands)(this) + private[sbt] lazy val nonMultiParsers = Command.combine(definedCommands)(this) + lazy val combinedParser = (BasicCommands.multiApplied(this) | nonMultiParsers).failOnException def source: Option[CommandSource] = currentCommand match { diff --git a/main/src/main/scala/sbt/Main.scala b/main/src/main/scala/sbt/Main.scala index c883c42dd..f36fc1322 100644 --- a/main/src/main/scala/sbt/Main.scala +++ b/main/src/main/scala/sbt/Main.scala @@ -226,7 +226,6 @@ object BuiltinCommands { export, boot, initialize, - BasicCommands.multi, act, continuous, clearCaches, diff --git a/sbt/src/sbt-test/actions/multi-command/build.sbt b/sbt/src/sbt-test/actions/multi-command/build.sbt index 0aff2d364..bff6d9d45 100644 --- a/sbt/src/sbt-test/actions/multi-command/build.sbt +++ b/sbt/src/sbt-test/actions/multi-command/build.sbt @@ -12,3 +12,5 @@ taskThatFails := { throw new IllegalArgumentException("") () } + +checkInput := checkInputImpl.evaluated \ No newline at end of file diff --git a/sbt/src/sbt-test/actions/multi-command/project/Build.scala b/sbt/src/sbt-test/actions/multi-command/project/Build.scala index 5190dc191..06176933e 100644 --- a/sbt/src/sbt-test/actions/multi-command/project/Build.scala +++ b/sbt/src/sbt-test/actions/multi-command/project/Build.scala @@ -1,11 +1,14 @@ import sbt._ +import sbt.internal.util.complete.Parser._ + object Build { private[this] var string: String = "" private[this] val stringFile = file("string.txt") val setStringValue = inputKey[Unit]("set a global string to a value") val checkStringValue = inputKey[Unit]("check the value of a global") val taskThatFails = taskKey[Unit]("this should fail") + val checkInput = inputKey[Unit]("this should extract arguments that are semicolon delimited") def setStringValueImpl: Def.Initialize[InputTask[Unit]] = Def.inputTask { string = Def.spaceDelimited().parsed.mkString(" ").trim IO.write(stringFile, string) @@ -15,4 +18,9 @@ object Build { assert(string == actual) assert(IO.read(stringFile) == string) } + + def checkInputImpl: Def.Initialize[InputTask[Unit]] = Def.inputTask { + val actual = (charClass(_ != ';').+ <~ ';'.?).map(_.mkString.trim).+.parsed + assert(actual == Seq("foo")) + } } diff --git a/sbt/src/sbt-test/actions/multi-command/test b/sbt/src/sbt-test/actions/multi-command/test index 8bc828c03..f2626b4d6 100644 --- a/sbt/src/sbt-test/actions/multi-command/test +++ b/sbt/src/sbt-test/actions/multi-command/test @@ -19,3 +19,10 @@ -> setStringValue foo; taskThatFails; setStringValue bar > checkStringValue foo + +# this fails even though the checkInput parser would parse the input into Seq("foo", "bar") +-> checkInput foo; bar + +> checkInput foo + +> compile; checkInput foo