Fix Invalid Comment Edge Case

According to [the XML specification](https://www.w3.org/TR/xml11//#IDA5CES "W3 XML") XML comments can not end in `--->`. Prior to this commit creating a comment of the form `Comment("invalid-")` would yield no error though it should, since it is rendered as `<!--invalid--->`. This commit makes such `String` values yield an `IllegalArgumentException`.

A companion object was also added with two helper methods, `validated` and `validatedOpt`, which allow for a user of the library to check if a `String` is valid without having to `catch` the `IllegalArgumentException`.

Author: isomarcte

shared/src/main/scala/scala/xml/Comment.scala

@@ -14,6 +14,8 @@ package xml
  *
  * @author Burak Emir
  * @param commentText the text contained in this node, may not contain "--"
+ *        and the final character may not be `-` to prevent a closing span of `-->`
+ *        which is invalid. [[https://www.w3.org/TR/xml11//#IDA5CES]]
  */
 case class Comment(commentText: String) extends SpecialNode {
 
@@ -22,12 +24,81 @@ case class Comment(commentText: String) extends SpecialNode {
   final override def doCollectNamespaces = false
   final override def doTransform = false
 
-  if (commentText contains "--")
-    throw new IllegalArgumentException("text contains \"--\"")
+  // Validate the commentText. We call the `Comment.validate` method because
+  // that shares code with the more safe constructs, e.g. `Comment.validated`
+  // and `Comment.validatedOpt`.
+  Comment.validate(commentText).fold(
+    ()
+  )(e =>
+    throw e
+  )
 
   /**
    * Appends &quot;<!-- text -->&quot; to this string buffer.
    */
   override def buildString(sb: StringBuilder) =
     sb append "<!--" append commentText append "-->"
 }
+
+// We have to extends `scala.runtime.AbstractFunction1[String, Comment]` and
+// override `apply` if we don't want to break binary compatibility.
+object Comment extends scala.runtime.AbstractFunction1[String, Comment]{
+
+  override final def apply(commentText: String): Comment =
+    new Comment(commentText)
+
+  /** Validates if a given `String` can be a valid XML comment.
+    *
+    * There are two related cases where a `String` is invalid. The first is
+    * when the `String` `"--"` occurs in the body of the text. The second is
+    * when the final character in the comment is `'-'`. In the second case
+    * this is invalid because it would yield a XML encoding of `-->` for the
+    * final three characters of the rendered comment which invalidates the
+    * first rule of not having `"--"` in the comment.
+    *
+    * @see [[https://www.w3.org/TR/xml11//#IDA5CES]]
+    */
+  private def validate(commentText: String): Option[Throwable] =
+    if (commentText.isEmpty) {
+      None
+    } else if (commentText.contains("--")) {
+      Some(new IllegalArgumentException("text contains \"--\""))
+    } else if (commentText.charAt(commentText.length - 1) == '-') {
+      Some(new IllegalArgumentException("The final character of a XML comment may not be '-'. See https://www.w3.org/TR/xml11//#IDA5CES"))
+    } else {
+      None
+    }
+
+  /** Validates if a given `String` can be a valid XML comment, yielding a
+    * `Left` `Throwable` if it is invalid.
+    *
+    * There are two related cases where a `String` is invalid. The first is
+    * when the `String` `"--"` occurs in the body of the text. The second is
+    * when the final character in the comment is `'-'`. In the second case
+    * this is invalid because it would yield a XML encoding of `-->` for the
+    * final three characters of the rendered comment which invalidates the
+    * first rule of not having `"--"` in the comment.
+    *
+    * @see [[https://www.w3.org/TR/xml11//#IDA5CES]]
+    */
+  def validated(commentText: String): Either[Throwable, Comment] =
+    validate(commentText).fold(
+      Right(Comment(commentText)): Either[Throwable, Comment]
+    )(t => Left(t))
+
+
+  /** Validates if a given `String` can be a valid XML comment, `None` if it is
+    * invalid.
+    *
+    * There are two related cases where a `String` is invalid. The first is
+    * when the `String` `"--"` occurs in the body of the text. The second is
+    * when the final character in the comment is `'-'`. In the second case
+    * this is invalid because it would yield a XML encoding of `-->` for the
+    * final three characters of the rendered comment which invalidates the
+    * first rule of not having `"--"` in the comment.
+    *
+    * @see [[https://www.w3.org/TR/xml11//#IDA5CES]]
+    */
+  def validatedOpt(commentText: String): Option[Comment] =
+    validated(commentText).toOption
+}

shared/src/test/scala/scala/xml/CommentTest.scala

@@ -0,0 +1,38 @@
+package scala.xml
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+final class CommentTest {
+
+  @Test(expected=classOf[IllegalArgumentException])
+  def invalidCommentWithTwoDashes: Unit = {
+    val invalid: String = "invalid--comment"
+    assertTrue(Comment.validated(invalid).isLeft)
+    assertTrue(Comment.validatedOpt(invalid).isEmpty)
+    Comment(invalid)
+  }
+
+  @Test(expected=classOf[IllegalArgumentException])
+  def invalidCommentWithFinalDash: Unit = {
+    val invalid: String = "invalid comment-"
+    assertTrue(Comment.validated(invalid).isLeft)
+    assertTrue(Comment.validatedOpt(invalid).isEmpty)
+    Comment(invalid)
+  }
+
+  @Test
+  def validCommentWithDash: Unit = {
+    val valid: String = "valid-comment"
+    assertEquals(Right(Comment("valid-comment")), Comment.validated(valid))
+    assertEquals(Some(Comment("valid-comment")), Comment.validatedOpt(valid))
+  }
+
+  @Test
+  def validEmptyComment: Unit = {
+    val valid: String = ""
+    assertEquals(Right(Comment("")), Comment.validated(valid))
+    assertEquals(Some(Comment("")), Comment.validatedOpt(valid))
+  }
+}