Merge pull request #119 from chillu/urlsegment-multibyte-fix

Urlsegment multibyte fix

code/controllers/ContentController.php

@@ -146,7 +146,7 @@ class ContentController extends Controller {
 			if(class_exists('Translatable')) Translatable::disable_locale_filter();
 			// look for a page with this URLSegment
 			$child = $this->model->SiteTree->where(sprintf (
-				"\"ParentID\" = %s AND \"URLSegment\" = '%s'", $this->ID, Convert::raw2sql($action)
+				"\"ParentID\" = %s AND \"URLSegment\" = '%s'", $this->ID, Convert::raw2sql(rawurlencode($action))
 			))->First();
 			if(class_exists('Translatable')) Translatable::enable_locale_filter();
 			

code/controllers/ModelAsController.php

@@ -87,14 +87,14 @@ class ModelAsController extends Controller implements NestedController {
 		if(!$URLSegment = $request->param('URLSegment')) {
 			throw new Exception('ModelAsController->getNestedController(): was not passed a URLSegment value.');
 		}
-
+		
 		// Find page by link, regardless of current locale settings
 		if(class_exists('Translatable')) Translatable::disable_locale_filter();
 		$sitetree = DataObject::get_one(
 			'SiteTree', 
 			sprintf(
 				'"URLSegment" = \'%s\' %s', 
-				Convert::raw2sql($URLSegment), 
+				Convert::raw2sql(rawurlencode($URLSegment)), 
 				(SiteTree::nested_urls() ? 'AND "ParentID" = 0' : null)
 			)
 		);
@@ -149,7 +149,7 @@ class ModelAsController extends Controller implements NestedController {
 	 * @return SiteTree
 	 */
 	static function find_old_page($URLSegment,$parentID = 0, $ignoreNestedURLs = false) {
-		$URLSegment = Convert::raw2sql($URLSegment);
+		$URLSegment = Convert::raw2sql(rawurlencode($URLSegment));
 		
 		$useParentIDFilter = SiteTree::nested_urls() && $parentID;
 				

code/model/SiteTree.php

@@ -1,10 +1,16 @@
 <?php
 /**
  * Basic data-object representing all pages within the site tree.
- * This data-object takes care of the heirachy.  All page types that live within the heirachy
- * should inherit from this.
- *
+ * This data-object takes care of the heirachy. All page types that live within the hierarchy should inherit from this.
  * In addition, it contains a number of static methods for querying the site tree.
+ * 
+ * <h2>URLs</h2>
+ * A page is identified during request handling via its "URLSegment" database column. 
+ * As pages can be nested, the full path of a URL might contain multiple segments.
+ * Each segment is stored in its filtered representation (through {@link URLSegmentFilter}).
+ * The full path is constructed via {@link Link()}, {@link RelativeLink()} and {@link AbsoluteLink()}.
+ * You can allow these segments to contain multibyte characters through {@link URLSegmentFilter::$default_allow_multibyte}.
+ * 
  * @package cms
  */
 class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvider,CMSPreviewable {
@@ -1378,7 +1384,9 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 		// If there is no URLSegment set, generate one from Title
 		if((!$this->URLSegment || $this->URLSegment == 'new-page') && $this->Title) {
 			$this->URLSegment = $this->generateURLSegment($this->Title);
-		} else if($this->isChanged('URLSegment')) {
+		} else if($this->isChanged('URLSegment', 2)) {
+			// Do a strict check on change level, to avoid double encoding caused by
+			// bogus changes through forceChange()
 			$filter = URLSegmentFilter::create();
 			$this->URLSegment = $filter->filter($this->URLSegment);
 			// If after sanitising there is no URLSegment, give it a reasonable default

code/staticpublisher/FilesystemPublisher.php

@@ -70,7 +70,11 @@ class FilesystemPublisher extends StaticPublisher {
 	 * Transforms relative or absolute URLs to their static path equivalent.
 	 * This needs to be the same logic that's used to look up these paths through
 	 * framework/static-main.php. Does not include the {@link $destFolder} prefix.
-	 * Replaces various special characters in the resulting filename similar to {@link SiteTree::generateURLSegment()}.
+	 * 
+	 * URL filtering will have already taken place for direct SiteTree links via SiteTree->generateURLSegment()).
+	 * For all other links (e.g. custom controller actions), we assume that they're pre-sanitized
+	 * to suit the filesystem needs, as its impossible to sanitize them without risking to break
+	 * the underlying naming assumptions in URL routing (e.g. controller method names).
 	 * 
 	 * Examples (without $domain_based_caching):
 	 *  - http://mysite.com/mywebroot/ => /index.html (assuming your webroot is in a subfolder)
@@ -89,21 +93,21 @@ class FilesystemPublisher extends StaticPublisher {
 	function urlsToPaths($urls) {
 		$mappedUrls = array();
 		foreach($urls as $url) {
+
+			// parse_url() is not multibyte safe, see https://bugs.php.net/bug.php?id=52923.
+			// We assume that the URL hsa been correctly encoded either on storage (for SiteTree->URLSegment),
+			// or through URL collection (for controller method names etc.).
 			$urlParts = @parse_url($url);
 			
 			// Remove base folders from the URL if webroot is hosted in a subfolder (same as static-main.php)
 			$path = isset($urlParts['path']) ? $urlParts['path'] : '';
-			if(substr(strtolower($path), 0, strlen(BASE_URL)) == strtolower(BASE_URL)) {
-				$urlSegment = substr($path, strlen(BASE_URL));
+			if(mb_substr(mb_strtolower($path), 0, mb_strlen(BASE_URL)) == mb_strtolower(BASE_URL)) {
+				$urlSegment = mb_substr($path, mb_strlen(BASE_URL));
 			} else {
 				$urlSegment = $path;
 			}
 
-			// perform similar transformations to SiteTree::generateURLSegment()
-			$urlSegment = str_replace('&','-and-',$urlSegment);
-			$urlSegment = str_replace('&','-and-',$urlSegment);
-			$urlSegment = preg_replace('/[^A-Za-z0-9\/-]+/', '-', $urlSegment);
-			$urlSegment = preg_replace('/-+/', '-', $urlSegment);
+			// Normalize URLs
 			$urlSegment = trim($urlSegment, '/');
 
 			$filename = $urlSegment ? "$urlSegment.$this->fileExtension" : "index.$this->fileExtension";

tests/model/SiteTreeTest.php

@@ -720,6 +720,26 @@ class SiteTreeTest extends SapphireTest {
 		$sitetree->URLSegment = 'valid';
 		$this->assertTrue($sitetree->validURLSegment(), 'Valid URLSegment values are allowed');
 	}
+
+	public function testURLSegmentMultiByte() {
+		$origAllow = URLSegmentFilter::$default_allow_multibyte;
+		URLSegmentFilter::$default_allow_multibyte = true;
+		$sitetree = new SiteTree();
+		$sitetree->write();
+
+		$sitetree->URLSegment = 'brötchen';
+		$sitetree->write();
+		$sitetree = DataObject::get_by_id('SiteTree', $sitetree->ID, false);
+		$this->assertEquals($sitetree->URLSegment, rawurlencode('brötchen'));
+
+		$sitetree->publish('Stage', 'Live');
+		$sitetree = DataObject::get_by_id('SiteTree', $sitetree->ID, false);
+		$this->assertEquals($sitetree->URLSegment, rawurlencode('brötchen'));
+		$sitetreeLive = Versioned::get_one_by_stage('SiteTree', 'Live', '"SiteTree"."ID" = ' .$sitetree->ID, false);
+		$this->assertEquals($sitetreeLive->URLSegment, rawurlencode('brötchen'));
+
+		URLSegmentFilter::$default_allow_multibyte = $origAllow;
+	}
 	
 	public function testVersionsAreCreated() {
 		$p = new Page();

tests/staticpublisher/FilesystemPublisherTest.php

@@ -78,7 +78,7 @@ class FilesystemPublisherTest extends SapphireTest {
 			'Nested URLsegment path mapping'
 		);
 	}
-	
+
 	function testUrlsToPathsWithDomainBasedCaching() {
 		$origDomainBasedCaching = FilesystemPublisher::$domain_based_caching;
 		FilesystemPublisher::$domain_based_caching = true;