From e6cdde31c6a91424e40cc13beb9ab1b08cb06b44 Mon Sep 17 00:00:00 2001 From: Ondra Pelech Date: Sun, 20 Nov 2022 21:44:34 +0100 Subject: [PATCH] Fix Parser.parseDocuments (#346) Co-authored-by: Luca Violanti fixes https://github.com/circe/circe-yaml/issues/343 closes https://github.com/circe/circe-yaml/pull/342 --- .github/workflows/ci.yml | 1 + build.sbt | 1 + .../scala/io/circe/yaml/v12/ParserImpl.scala | 9 +- .../scala/io/circe/yaml/v12/ParserTests.scala | 99 ++++++++++++++++++- .../src/main/scala/io/circe/yaml/Parser.scala | 12 ++- .../scala/io/circe/yaml/ParserTests.scala | 97 +++++++++++++++++- 6 files changed, 211 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1fc2081a..9278b40b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,6 +26,7 @@ jobs: build: name: Build and Test strategy: + fail-fast: false matrix: os: [ubuntu-latest] scala: [2.12.17, 2.13.10, 3.2.1] diff --git a/build.sbt b/build.sbt index 276a7983..fdd9e1b8 100644 --- a/build.sbt +++ b/build.sbt @@ -3,6 +3,7 @@ ThisBuild / circeRootOfCodeCoverage := None ThisBuild / startYear := Some(2016) ThisBuild / scalafixScalaBinaryVersion := "2.12" ThisBuild / tlFatalWarningsInCi := false //TODO: ... fix this someday +ThisBuild / githubWorkflowBuildMatrixFailFast := Some(false) val Versions = new { val circe = "0.14.3" diff --git a/circe-yaml-v12/src/main/scala/io/circe/yaml/v12/ParserImpl.scala b/circe-yaml-v12/src/main/scala/io/circe/yaml/v12/ParserImpl.scala index 8bcbe7cf..7300feef 100644 --- a/circe-yaml-v12/src/main/scala/io/circe/yaml/v12/ParserImpl.scala +++ b/circe-yaml-v12/src/main/scala/io/circe/yaml/v12/ParserImpl.scala @@ -46,7 +46,10 @@ class ParserImpl(settings: LoadSettings) extends common.Parser { def parse(yaml: String): Either[ParsingFailure, Json] = parse(new StringReader(yaml)) - def parseDocuments(yaml: Reader): Stream[Either[ParsingFailure, Json]] = parseStream(yaml).map(yamlToJson) + def parseDocuments(yaml: Reader): Stream[Either[ParsingFailure, Json]] = parseStream(yaml) match { + case Left(error) => Stream(Left(error)) + case Right(stream) => stream.map(yamlToJson) + } def parseDocuments(yaml: String): Stream[Either[ParsingFailure, Json]] = parseDocuments(new StringReader(yaml)) private[this] def asScala[T](ot: Optional[T]): Option[T] = @@ -65,8 +68,8 @@ class ParserImpl(settings: LoadSettings) extends common.Parser { case Right(Some(value)) => Right(value) } - private[this] def parseStream(reader: Reader) = - createComposer(reader).asScala.toStream + private[this] def parseStream(reader: Reader): Either[ParsingFailure, Stream[Node]] = + Either.catchNonFatal(createComposer(reader).asScala.toStream).leftMap(err => ParsingFailure(err.getMessage, err)) final def decode[A: Decoder](input: Reader): Either[Error, A] = finishDecode(parse(input)) diff --git a/circe-yaml-v12/src/test/scala/io/circe/yaml/v12/ParserTests.scala b/circe-yaml-v12/src/test/scala/io/circe/yaml/v12/ParserTests.scala index 93127eec..fcb58e36 100644 --- a/circe-yaml-v12/src/test/scala/io/circe/yaml/v12/ParserTests.scala +++ b/circe-yaml-v12/src/test/scala/io/circe/yaml/v12/ParserTests.scala @@ -16,15 +16,18 @@ package io.circe.yaml.v12 +import io.circe.Json import io.circe.syntax._ import org.scalatest.EitherValues import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers +import java.io.StringReader + class ParserTests extends AnyFlatSpec with Matchers with EitherValues { // the laws should do a pretty good job of surfacing errors; these are mainly to ensure test coverage - "Parser" should "fail on invalid tagged numbers" in { + "Parser.parse" should "fail on invalid tagged numbers" in { assert(parser.parse("!!int 12foo").isLeft) } @@ -137,4 +140,98 @@ class ParserTests extends AnyFlatSpec with Matchers with EitherValues { .isLeft ) } + + "Parser.parseDocuments" should "fail on invalid tagged numbers" in { + val result = parser.parseDocuments(new StringReader("!!int 12foo")).toList + assert(result.size == 1) + assert(result.head.isLeft) + } + + it should "fail to parse complex keys" in { + val result = parser + .parseDocuments(new StringReader(""" + |? - foo + | - bar + |: 1""".stripMargin)) + .toList + assert(result.size == 1) + assert(result.head.isLeft) + } + + it should "fail to parse invalid YAML" in { + val result = parser.parseDocuments(new StringReader("""foo: - bar""")).toList + assert(result.size == 1) + assert(result.head.isLeft) + assert(result.head.isInstanceOf[Either[io.circe.ParsingFailure, Json]]) + } + + it should "parse yes as true" in { + val result = parser.parseDocuments(new StringReader("""foo: yes""")).toList + assert(result.size == 1) + assert(result.head.isRight) + } + + it should "parse hexadecimal as strings" in { + val result = parser.parseDocuments(new StringReader("""[0xFF, 0xff, 0xab_cd]""")).toList + assert(result.size == 1) + assert(result.head.contains(Seq("0xFF", "0xff", "0xab_cd").asJson.asJson)) + } + + it should "parse decimal with underscore breaks as strings" in { + val result = parser.parseDocuments(new StringReader("""foo: 1_000_000""")).toList + assert(result.size == 1) + assert(result.head.contains(Map("foo" -> "1_000_000").asJson)) + } + + it should "parse empty string as 0 documents" in { + val result = parser.parseDocuments(new StringReader("")).toList + assert(result.isEmpty) + } + + it should "parse blank string as 0 documents" in { + val result = parser.parseDocuments(new StringReader(" ")).toList + assert(result.isEmpty) + } + + it should "parse aliases" in { + val result = parser + .parseDocuments( + new StringReader( + """ + | aliases: + | - &alias1 + | foo: + | bar + | baz: + | - *alias1 + | - *alias1 + |""".stripMargin + ) + ) + .toList + assert(result.size == 1) + assert(result.head.isRight) + } + + it should "fail to parse too many aliases" in { + val result = + Parser + .make(Parser.Config(maxAliasesForCollections = 1)) + .parseDocuments( + new StringReader( + """ + | aliases: + | - &alias1 + | foo: + | bar + | baz: + | - *alias1 + | - *alias1 + |""".stripMargin + ) + ) + .toList + assertResult(1)(result.size) + assert(result.head.isLeft) + } } diff --git a/circe-yaml/src/main/scala/io/circe/yaml/Parser.scala b/circe-yaml/src/main/scala/io/circe/yaml/Parser.scala index 23b15ded..14d5de0a 100644 --- a/circe-yaml/src/main/scala/io/circe/yaml/Parser.scala +++ b/circe-yaml/src/main/scala/io/circe/yaml/Parser.scala @@ -52,14 +52,20 @@ final case class Parser( def parse(yaml: String): Either[ParsingFailure, Json] = parse(new StringReader(yaml)) - def parseDocuments(yaml: Reader): Stream[Either[ParsingFailure, Json]] = parseStream(yaml).map(yamlToJson) + def parseDocuments(yaml: Reader): Stream[Either[ParsingFailure, Json]] = parseStream(yaml) match { + case Left(error) => Stream(Left(error)) + case Right(stream) => stream.map(yamlToJson) + } + def parseDocuments(yaml: String): Stream[Either[ParsingFailure, Json]] = parseDocuments(new StringReader(yaml)) private[this] def parseSingle(reader: Reader): Either[ParsingFailure, Node] = Either.catchNonFatal(new Yaml(loaderOptions).compose(reader)).leftMap(err => ParsingFailure(err.getMessage, err)) - private[this] def parseStream(reader: Reader): Stream[Node] = - new Yaml(loaderOptions).composeAll(reader).asScala.toStream + private[this] def parseStream(reader: Reader): Either[ParsingFailure, Stream[Node]] = + Either + .catchNonFatal(new Yaml(loaderOptions).composeAll(reader).asScala.toStream) + .leftMap(err => ParsingFailure(err.getMessage, err)) final def decode[A: Decoder](input: Reader): Either[Error, A] = finishDecode(parse(input)) diff --git a/circe-yaml/src/test/scala/io/circe/yaml/ParserTests.scala b/circe-yaml/src/test/scala/io/circe/yaml/ParserTests.scala index 35ed435f..4b4bb18f 100644 --- a/circe-yaml/src/test/scala/io/circe/yaml/ParserTests.scala +++ b/circe-yaml/src/test/scala/io/circe/yaml/ParserTests.scala @@ -22,10 +22,12 @@ import org.scalatest.EitherValues import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers +import java.io.StringReader + class ParserTests extends AnyFlatSpec with Matchers with EitherValues { // the laws should do a pretty good job of surfacing errors; these are mainly to ensure test coverage - "Parser" should "fail on invalid tagged numbers" in { + "Parser.parse" should "fail on invalid tagged numbers" in { assert(parser.parse("!!int 12foo").isLeft) } @@ -138,4 +140,97 @@ class ParserTests extends AnyFlatSpec with Matchers with EitherValues { .isLeft ) } + + "Parser.parseDocuments" should "fail on invalid tagged numbers" in { + val result = parser.parseDocuments(new StringReader("!!int 12foo")).toList + assert(result.size == 1) + assert(result.head.isLeft) + } + + it should "fail to parse complex keys" in { + val result = parser + .parseDocuments(new StringReader(""" + |? - foo + | - bar + |: 1""".stripMargin)) + .toList + assert(result.size == 1) + assert(result.head.isLeft) + } + + it should "fail to parse invalid YAML" in { + val result = parser.parseDocuments(new StringReader("""foo: - bar""")).toList + assert(result.size == 1) + assert(result.head.isLeft) + assert(result.head.isInstanceOf[Either[io.circe.ParsingFailure, Json]]) + } + + it should "parse yes as true" in { + val result = parser.parseDocuments(new StringReader("""foo: yes""")).toList + assert(result.size == 1) + assert(result.head.isRight) + } + + it should "parse hexadecimal" in { + val result = parser.parseDocuments(new StringReader("""[0xFF, 0xff, 0xab_cd]""")).toList + assert(result.size == 1) + assert(result.head.contains(Seq(0xff, 0xff, 0xabcd).asJson)) + } + + it should "parse decimal with underscore breaks" in { + val result = parser.parseDocuments(new StringReader("""foo: 1_000_000""")).toList + assert(result.size == 1) + assert(result.head.contains(Map("foo" -> 1000000).asJson)) + } + + it should "parseDocuments empty string as 0 documents" in { + val result = parser.parseDocuments(new StringReader("")).toList + assert(result.isEmpty) + } + + it should "parseDocuments blank string as 0 documents" in { + val result = parser.parseDocuments(new StringReader(" ")).toList + assert(result.isEmpty) + } + + it should "parse aliases" in { + val result = parser + .parseDocuments( + new StringReader( + """ + | aliases: + | - &alias1 + | foo: + | bar + | baz: + | - *alias1 + | - *alias1 + |""".stripMargin + ) + ) + .toList + assert(result.size == 1) + assert(result.head.isRight) + } + + it should "fail to parse too many aliases" in { + val result = + Parser(maxAliasesForCollections = 1) + .parseDocuments( + new StringReader( + """ + | aliases: + | - &alias1 + | foo: + | bar + | baz: + | - *alias1 + | - *alias1 + |""".stripMargin + ) + ) + .toList + assertResult(1)(result.size) + assert(result.head.isLeft) + } }