From 85034abca5fa4879ab206ccb010ab1c37d8b87f3 Mon Sep 17 00:00:00 2001
From: Kunal Mehta <legoktm@member.fsf.org>
Date: Tue, 16 Aug 2016 14:58:15 -0700
Subject: [PATCH] content: Refactor normalization of line endings code

The code that normalizes line endings ("\r\n" and "\r" to "\n") and
trims trailing whitespace is buried in Parser::preSaveTransform(), and
was duplicated to TextContent in 96b6afb31dfcff, as non-wikitext content
models should still be normalizing line endings.

This splits the duplicated code into
TextContent::normalizeLineEndings(), and utilize it in the Parser.
Additionally, expand the documentation of
TextContent::preSaveTransform() to document that subclasses should make
sure they normalize line endings during the PST stage.

And remove a useless rtrim() call from WikitextContent that did nothing.

Change-Id: I9094c671d4bbd23d75436f8f1d682d6dd6e6d2fc
---
 includes/content/JsonContent.php              |  2 +-
 includes/content/TextContent.php              | 24 ++++++++++++++---
 includes/content/WikitextContent.php          |  1 -
 includes/parser/Parser.php                    |  9 ++++---
 .../includes/content/TextContentTest.php      | 26 +++++++++++++++++++
 5 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/includes/content/JsonContent.php b/includes/content/JsonContent.php
index 40d9277470..14c81823cf 100644
--- a/includes/content/JsonContent.php
+++ b/includes/content/JsonContent.php
@@ -86,7 +86,7 @@ class JsonContent extends TextContent {
 			return $this;
 		}
 
-		return new static( $this->beautifyJSON() );
+		return new static( self::normalizeLineEndings( $this->beautifyJSON() ) );
 	}
 
 	/**
diff --git a/includes/content/TextContent.php b/includes/content/TextContent.php
index de650b9194..7bb4def2a6 100644
--- a/includes/content/TextContent.php
+++ b/includes/content/TextContent.php
@@ -147,9 +147,28 @@ class TextContent extends AbstractContent {
 		}
 	}
 
+	/**
+	 * Do a "\r\n" -> "\n" and "\r" -> "\n" transformation
+	 * as well as trim trailing whitespace
+	 *
+	 * This was formerly part of Parser::preSaveTransform, but
+	 * for non-wikitext content models they probably still want
+	 * to normalize line endings without all of the other PST
+	 * changes.
+	 *
+	 * @since 1.28
+	 * @param $text
+	 * @return string
+	 */
+	public static function normalizeLineEndings( $text ) {
+		return str_replace( [ "\r\n", "\r" ], "\n", rtrim( $text ) );
+	}
+
 	/**
 	 * Returns a Content object with pre-save transformations applied.
-	 * This implementation just trims trailing whitespace and normalizes newlines.
+	 *
+	 * At a minimum, subclasses should make sure to call TextContent::normalizeLineEndings()
+	 * either directly or part of Parser::preSaveTransform().
 	 *
 	 * @param Title $title
 	 * @param User $user
@@ -159,8 +178,7 @@ class TextContent extends AbstractContent {
 	 */
 	public function preSaveTransform( Title $title, User $user, ParserOptions $popts ) {
 		$text = $this->getNativeData();
-		$pst = rtrim( $text );
-		$pst = str_replace( [ "\r\n", "\r" ], "\n", $pst );
+		$pst = self::normalizeLineEndings( $text );
 
 		return ( $text === $pst ) ? $this : new static( $pst, $this->getModel() );
 	}
diff --git a/includes/content/WikitextContent.php b/includes/content/WikitextContent.php
index a63819dd42..92967282c4 100644
--- a/includes/content/WikitextContent.php
+++ b/includes/content/WikitextContent.php
@@ -138,7 +138,6 @@ class WikitextContent extends TextContent {
 
 		$text = $this->getNativeData();
 		$pst = $wgParser->preSaveTransform( $text, $title, $user, $popts );
-		rtrim( $pst );
 
 		return ( $text === $pst ) ? $this : new static( $pst );
 	}
diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
index 38eb621936..b116bd4451 100644
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@ -4364,7 +4364,11 @@ class Parser {
 		$this->startParse( $title, $options, self::OT_WIKI, $clearState );
 		$this->setUser( $user );
 
-		$text = str_replace( [ "\r\n", "\r" ], "\n", $text );
+		// We still normalize line endings for backwards-compatibility
+		// with other code that just calls PST, but this should already
+		// be handled in TextContent subclasses
+		$text = TextContent::normalizeLineEndings( $text );
+
 		if ( $options->getPreSaveTransform() ) {
 			$text = $this->pstPass2( $text, $user );
 		}
@@ -4442,9 +4446,6 @@ class Parser {
 			$text = preg_replace( $p2, '[[\\1]]', $text );
 		}
 
-		# Trim trailing whitespace
-		$text = rtrim( $text );
-
 		return $text;
 	}
 
diff --git a/tests/phpunit/includes/content/TextContentTest.php b/tests/phpunit/includes/content/TextContentTest.php
index b4ae765f72..b290f8f281 100644
--- a/tests/phpunit/includes/content/TextContentTest.php
+++ b/tests/phpunit/includes/content/TextContentTest.php
@@ -459,4 +459,30 @@ class TextContentTest extends MediaWikiLangTestCase {
 			$this->assertEquals( $expectedNative, $converted->getNativeData() );
 		}
 	}
+
+	/**
+	 * @covers TextContent::normalizeLineEndings
+	 * @dataProvider provideNormalizeLineEndings
+	 */
+	public function testNormalizeLineEndings( $input, $expected ) {
+		$this->assertEquals( $expected, TextContent::normalizeLineEndings( $input ) );
+	}
+
+	public static function provideNormalizeLineEndings() {
+		return [
+			[
+				"Foo\r\nbar",
+				"Foo\nbar"
+			],
+			[
+				"Foo\rbar",
+				"Foo\nbar"
+			],
+			[
+				"Foobar\n  ",
+				"Foobar"
+			]
+		];
+	}
+
 }
-- 
2.20.1