Skip to content
This repository
Browse code

HTML escape context variables.

When creating HTML or js errors the context should be
escaped, as it is very possible that the context vars will contain
HTML.

Clean up some internals in Debugger::outputError().  There were
a few duplicate data structures, and $$ variables.

Fixes #2884
  • Loading branch information...
commit c6258fa68c09e785bedbc6e517b61c869f25f727 1 parent 0faaedf
Mark Story authored May 16, 2012
9  lib/Cake/Test/Case/Utility/DebuggerTest.php
@@ -140,8 +140,8 @@ public function testOutput() {
140 140
 			'a' => array(
141 141
 				'href' => "javascript:void(0);",
142 142
 				'onclick' => "preg:/document\.getElementById\('cakeErr[a-z0-9]+\-trace'\)\.style\.display = " .
143  
-							 "\(document\.getElementById\('cakeErr[a-z0-9]+\-trace'\)\.style\.display == 'none'" .
144  
-							 " \? '' \: 'none'\);/"
  143
+					 "\(document\.getElementById\('cakeErr[a-z0-9]+\-trace'\)\.style\.display == 'none'" .
  144
+					 " \? '' \: 'none'\);/"
145 145
 			),
146 146
 			'b' => array(), 'Notice', '/b', ' (8)',
147 147
 		));
@@ -149,6 +149,7 @@ public function testOutput() {
149 149
 		$this->assertRegExp('/Undefined variable:\s+buzz/', $result[1]);
150 150
 		$this->assertRegExp('/<a[^>]+>Code/', $result[1]);
151 151
 		$this->assertRegExp('/<a[^>]+>Context/', $result[2]);
  152
+		$this->assertContains('$wrong = &#039;&#039;', $result[3], 'Context should be HTML escaped.');
152 153
 	}
153 154
 
154 155
 /**
@@ -162,14 +163,14 @@ public function testChangeOutputFormats() {
162 163
 
163 164
 		Debugger::output('js', array(
164 165
 			'traceLine' => '{:reference} - <a href="txmt://open?url=file://{:file}' .
165  
-							 '&line={:line}">{:path}</a>, line {:line}'
  166
+				'&line={:line}">{:path}</a>, line {:line}'
166 167
 		));
167 168
 		$result = Debugger::trace();
168 169
 		$this->assertRegExp('/' . preg_quote('txmt://open?url=file://', '/') . '(\/|[A-Z]:\\\\)' . '/', $result);
169 170
 
170 171
 		Debugger::output('xml', array(
171 172
 			'error' => '<error><code>{:code}</code><file>{:file}</file><line>{:line}</line>' .
172  
-						 '{:description}</error>',
  173
+				 '{:description}</error>',
173 174
 			'context' => "<context>{:context}</context>",
174 175
 			'trace' => "<stack>{:trace}</stack>",
175 176
 		));
33  lib/Cake/Utility/Debugger.php
@@ -63,11 +63,13 @@ class Debugger {
63 63
 			'trace' => '<pre class="stack-trace">{:trace}</pre>',
64 64
 			'code' => '',
65 65
 			'context' => '',
66  
-			'links' => array()
  66
+			'links' => array(),
  67
+			'escapeContext' => true,
67 68
 		),
68 69
 		'html' => array(
69 70
 			'trace' => '<pre class="cake-error trace"><b>Trace</b> <p>{:trace}</p></pre>',
70  
-			'context' => '<pre class="cake-error context"><b>Context</b> <p>{:context}</p></pre>'
  71
+			'context' => '<pre class="cake-error context"><b>Context</b> <p>{:context}</p></pre>',
  72
+			'escapeContext' => true,
71 73
 		),
72 74
 		'txt' => array(
73 75
 			'error' => "{:error}: {:code} :: {:description} on line {:line} of {:path}\n{:info}",
@@ -716,7 +718,7 @@ public function outputError($data) {
716 718
 		$info = '';
717 719
 
718 720
 		foreach ((array)$data['context'] as $var => $value) {
719  
-			$context[] = "\${$var}\t=\t" . $this->exportVar($value, 1);
  721
+			$context[] = "\${$var} = " . $this->exportVar($value, 1);
720 722
 		}
721 723
 
722 724
 		switch ($this->_outputFormat) {
@@ -731,30 +733,29 @@ public function outputError($data) {
731 733
 		$data['trace'] = $trace;
732 734
 		$data['id'] = 'cakeErr' . uniqid();
733 735
 		$tpl = array_merge($this->_templates['base'], $this->_templates[$this->_outputFormat]);
734  
-		$insert = array('context' => join("\n", $context)) + $data;
735  
-
736  
-		$detect = array('context');
737 736
 
738 737
 		if (isset($tpl['links'])) {
739 738
 			foreach ($tpl['links'] as $key => $val) {
740  
-				if (in_array($key, $detect) && empty($insert[$key])) {
741  
-					continue;
742  
-				}
743  
-				$links[$key] = String::insert($val, $insert, $insertOpts);
  739
+				$links[$key] = String::insert($val, $data, $insertOpts);
744 740
 			}
745 741
 		}
746 742
 
747  
-		foreach (array('code', 'context', 'trace') as $key) {
748  
-			if (empty($$key) || !isset($tpl[$key])) {
  743
+		if (!empty($tpl['escapeContext'])) {
  744
+			$context = h($context);
  745
+		}
  746
+
  747
+		$infoData = compact('code', 'context', 'trace');
  748
+		foreach ($infoData as $key => $value) {
  749
+			if (empty($value) || !isset($tpl[$key])) {
749 750
 				continue;
750 751
 			}
751  
-			if (is_array($$key)) {
752  
-				$$key = join("\n", $$key);
  752
+			if (is_array($value)) {
  753
+				$value = join("\n", $value);
753 754
 			}
754  
-			$info .= String::insert($tpl[$key], compact($key) + $insert, $insertOpts);
  755
+			$info .= String::insert($tpl[$key], array($key => $value) + $data, $insertOpts);
755 756
 		}
756 757
 		$links = join(' ', $links);
757  
-		unset($data['context']);
  758
+
758 759
 		if (isset($tpl['callback']) && is_callable($tpl['callback'])) {
759 760
 			return call_user_func($tpl['callback'], $data, compact('links', 'info'));
760 761
 		}

0 notes on commit c6258fa

Please sign in to comment.
Something went wrong with that request. Please try again.