From 7b4df0e12e36332fc2b303d7dac2295f8d36c888 Mon Sep 17 00:00:00 2001
From: Brian Wolff <bawolff+wn@gmail.com>
Date: Wed, 8 Jul 2015 23:19:35 -0600
Subject: [PATCH] Do not encode '~' as %7E. Fixes redirect loop in chrome.

As of 155d555b83eca640, we now redirect variations on hex escapes
into their canonical form. This was causing '~' to be redirected
to %7E. However google chrome seems to canonicalize %7E back
into ~, causing a redirect loop.

RFC 3986 says ~ is unreserved, so not hex encoding it should be
fine. To quote: "For consistency, percent-encoded octets in the
ranges of...tilde (%7E) should not be created by URI producers"

Bug: T105265
Change-Id: I01556eee496e2fb540de8ff09c082c1fedddb5f7
---
 includes/GlobalFunctions.php                          | 11 ++++++++---
 resources/src/mediawiki/mediawiki.util.js             |  1 +
 tests/parser/parserTests.txt                          |  2 +-
 .../includes/GlobalFunctions/wfUrlencodeTest.php      |  2 ++
 .../suites/resources/mediawiki/mediawiki.util.test.js |  3 ++-
 5 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 97042fd1f0..00d3d3a3da 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -402,12 +402,17 @@ function wfRandomString( $length = 32 ) {
  *
  * ;:@&=$-_.+!*'(),
  *
+ * RFC 1738 says ~ is unsafe, however RFC 3986 considers it an unreserved
+ * character which should not be encoded. More importantly, google chrome
+ * always converts %7E back to ~, and converting it in this function can
+ * cause a redirect loop (T105265).
+ *
  * But + is not safe because it's used to indicate a space; &= are only safe in
  * paths and not in queries (and we don't distinguish here); ' seems kind of
  * scary; and urlencode() doesn't touch -_. to begin with.  Plus, although /
  * is reserved, we don't care.  So the list we unescape is:
  *
- * ;:@$!*(),/
+ * ;:@$!*(),/~
  *
  * However, IIS7 redirects fail when the url contains a colon (Bug 22709),
  * so no fancy : for IIS7.
@@ -426,7 +431,7 @@ function wfUrlencode( $s ) {
 	}
 
 	if ( is_null( $needle ) ) {
-		$needle = array( '%3B', '%40', '%24', '%21', '%2A', '%28', '%29', '%2C', '%2F' );
+		$needle = array( '%3B', '%40', '%24', '%21', '%2A', '%28', '%29', '%2C', '%2F', '%7E' );
 		if ( !isset( $_SERVER['SERVER_SOFTWARE'] ) ||
 			( strpos( $_SERVER['SERVER_SOFTWARE'], 'Microsoft-IIS/7' ) === false )
 		) {
@@ -437,7 +442,7 @@ function wfUrlencode( $s ) {
 	$s = urlencode( $s );
 	$s = str_ireplace(
 		$needle,
-		array( ';', '@', '$', '!', '*', '(', ')', ',', '/', ':' ),
+		array( ';', '@', '$', '!', '*', '(', ')', ',', '/', '~', ':' ),
 		$s
 	);
 
diff --git a/resources/src/mediawiki/mediawiki.util.js b/resources/src/mediawiki/mediawiki.util.js
index 13bf455933..93a1b3bfbc 100644
--- a/resources/src/mediawiki/mediawiki.util.js
+++ b/resources/src/mediawiki/mediawiki.util.js
@@ -82,6 +82,7 @@
 				.replace( /%29/g, ')' )
 				.replace( /%2C/g, ',' )
 				.replace( /%2F/g, '/' )
+				.replace( /%7E/g, '~' )
 				.replace( /%3A/g, ':' );
 		},
 
diff --git a/tests/parser/parserTests.txt b/tests/parser/parserTests.txt
index b3944fb99f..6700225c64 100644
--- a/tests/parser/parserTests.txt
+++ b/tests/parser/parserTests.txt
@@ -7075,7 +7075,7 @@ Link containing a tilde
 !! wikitext
 [[Foo~bar]]
 !! html/php
-<p><a href="/wiki/Foo%7Ebar" title="Foo~bar">Foo~bar</a>
+<p><a href="/wiki/Foo~bar" title="Foo~bar">Foo~bar</a>
 </p>
 !! html/parsoid
 <p><a rel="mw:WikiLink" href="./Foo~bar" title="Foo~bar">Foo~bar</a></p>
diff --git a/tests/phpunit/includes/GlobalFunctions/wfUrlencodeTest.php b/tests/phpunit/includes/GlobalFunctions/wfUrlencodeTest.php
index d11668b7c6..d4df7b0060 100644
--- a/tests/phpunit/includes/GlobalFunctions/wfUrlencodeTest.php
+++ b/tests/phpunit/includes/GlobalFunctions/wfUrlencodeTest.php
@@ -112,6 +112,8 @@ class WfUrlencodeTest extends MediaWikiTestCase {
 			### Other tests
 			// slash remain unchanged. %2F seems to break things
 			array( '/', '/' ),
+			// T105265
+			array( '~', '~' ),
 
 			// Other 'funnies' chars
 			array( '[]', '%5B%5D' ),
diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js
index 8a3e100dd7..b73d2e39cf 100644
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js
@@ -92,7 +92,7 @@
 		assert.equal( mw.util.rawurlencode( 'Test:A & B/Here' ), 'Test%3AA%20%26%20B%2FHere' );
 	} );
 
-	QUnit.test( 'wikiUrlencode', 10, function ( assert ) {
+	QUnit.test( 'wikiUrlencode', 11, function ( assert ) {
 		assert.equal( mw.util.wikiUrlencode( 'Test:A & B/Here' ), 'Test:A_%26_B/Here' );
 		// See also wfUrlencodeTest.php#provideURLS
 		$.each( {
@@ -102,6 +102,7 @@
 			':': ':',
 			';@$-_.!*': ';@$-_.!*',
 			'/': '/',
+			'~': '~',
 			'[]': '%5B%5D',
 			'<>': '%3C%3E',
 			'\'': '%27'
-- 
2.20.1