1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-20 20:40:56 +01:00

Add flat text assertions to Phabricator remarkup rules

Summary: Remarkup rules can not safely use arbitrary text in tag attributes,
because it may include tokens which are later replaced. Precedence rules
should prevent this in general. Use flat text assertions and adjust precedence
rules in cases where they may not prevent tokens from appearing in attributes.

Auditors: btrahan
This commit is contained in:
epriestley 2014-07-01 11:04:05 -07:00
parent ede6947dd1
commit 90e75d4e50
11 changed files with 61 additions and 45 deletions

View file

@ -8,10 +8,10 @@ return array(
'names' =>
array(
'core.pkg.css' => 'c2c68e64',
'core.pkg.js' => '834b4eda',
'core.pkg.js' => 'f8ec7ddc',
'darkconsole.pkg.js' => 'df001cab',
'differential.pkg.css' => '4a93db37',
'differential.pkg.js' => 'd1443567',
'differential.pkg.js' => '7528cfc9',
'diffusion.pkg.css' => '471bc9eb',
'diffusion.pkg.js' => 'bfc0737b',
'maniphest.pkg.css' => 'f5d89daf',
@ -362,7 +362,7 @@ return array(
'rsrc/js/application/differential/ChangesetViewManager.js' => 'd2907473',
'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => 'f2441746',
'rsrc/js/application/differential/behavior-add-reviewers-and-ccs.js' => 'e10f8e18',
'rsrc/js/application/differential/behavior-comment-jump.js' => '2bc7a5e0',
'rsrc/js/application/differential/behavior-comment-jump.js' => '4fdb476d',
'rsrc/js/application/differential/behavior-comment-preview.js' => '127f2018',
'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1',
'rsrc/js/application/differential/behavior-dropdown-menus.js' => '710f209e',
@ -473,7 +473,7 @@ return array(
'rsrc/js/core/behavior-object-selector.js' => '39841ead',
'rsrc/js/core/behavior-oncopy.js' => '2926fff2',
'rsrc/js/core/behavior-phabricator-nav.js' => '14d7a8b8',
'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => 'eff6a142',
'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => 'e32d14ab',
'rsrc/js/core/behavior-refresh-csrf.js' => '7814b593',
'rsrc/js/core/behavior-remarkup-preview.js' => 'f7379f45',
'rsrc/js/core/behavior-reorder-applications.js' => '76b9fc3e',
@ -567,7 +567,7 @@ return array(
'javelin-behavior-dashboard-tab-panel' => 'd4eecc63',
'javelin-behavior-device' => '03d6ed07',
'javelin-behavior-differential-add-reviewers-and-ccs' => 'e10f8e18',
'javelin-behavior-differential-comment-jump' => '2bc7a5e0',
'javelin-behavior-differential-comment-jump' => '4fdb476d',
'javelin-behavior-differential-diff-radios' => 'e1ff79b1',
'javelin-behavior-differential-dropdown-menus' => '710f209e',
'javelin-behavior-differential-edit-inline-comments' => '00861799',
@ -621,7 +621,7 @@ return array(
'javelin-behavior-phabricator-notification-example' => '7a9677fc',
'javelin-behavior-phabricator-object-selector' => '39841ead',
'javelin-behavior-phabricator-oncopy' => '2926fff2',
'javelin-behavior-phabricator-remarkup-assist' => 'eff6a142',
'javelin-behavior-phabricator-remarkup-assist' => 'e32d14ab',
'javelin-behavior-phabricator-reveal-content' => '60821bc7',
'javelin-behavior-phabricator-search-typeahead' => '5a376f34',
'javelin-behavior-phabricator-show-all-transactions' => '7c273581',
@ -1047,12 +1047,6 @@ return array(
3 => 'javelin-workflow',
4 => 'javelin-json',
),
'2bc7a5e0' =>
array(
0 => 'javelin-behavior',
1 => 'javelin-stratcom',
2 => 'javelin-dom',
),
'2cad29d1' =>
array(
0 => 'javelin-install',
@ -1225,6 +1219,12 @@ return array(
3 => 'javelin-stratcom',
4 => 'javelin-request',
),
'4fdb476d' =>
array(
0 => 'javelin-behavior',
1 => 'javelin-stratcom',
2 => 'javelin-dom',
),
'519705ea' =>
array(
0 => 'javelin-install',
@ -1952,6 +1952,16 @@ return array(
1 => 'javelin-stratcom',
2 => 'javelin-dom',
),
'e32d14ab' =>
array(
0 => 'javelin-behavior',
1 => 'javelin-stratcom',
2 => 'javelin-dom',
3 => 'phabricator-phtize',
4 => 'phabricator-textareautils',
5 => 'javelin-workflow',
6 => 'javelin-vector',
),
'e379b58e' =>
array(
0 => 'javelin-behavior',
@ -1995,16 +2005,6 @@ return array(
0 => 'javelin-install',
1 => 'javelin-util',
),
'eff6a142' =>
array(
0 => 'javelin-behavior',
1 => 'javelin-stratcom',
2 => 'javelin-dom',
3 => 'phabricator-phtize',
4 => 'phabricator-textareautils',
5 => 'javelin-workflow',
6 => 'javelin-vector',
),
'f0a41b9f' =>
array(
0 => 'javelin-behavior',

View file

@ -4,6 +4,10 @@ final class DivinerRemarkupRuleSymbol extends PhutilRemarkupRule {
const KEY_RULE_ATOM_REF = 'rule.diviner.atomref';
public function getPriority() {
return 40.0;
}
public function apply($text) {
// Grammar here is:
//
@ -132,7 +136,7 @@ final class DivinerRemarkupRuleSymbol extends PhutilRemarkupRule {
$link = $title;
}
} else if ($href) {
$link = phutil_tag(
$link = $this->newTag(
'a',
array(
'class' => 'atom-ref',
@ -140,7 +144,7 @@ final class DivinerRemarkupRuleSymbol extends PhutilRemarkupRule {
),
$title);
} else {
$link = phutil_tag(
$link = $this->newTag(
'span',
array(
'class' => 'atom-ref-invalid',

View file

@ -48,8 +48,8 @@ abstract class DoorkeeperRemarkupRule
} else {
$view = id(new PHUITagView())
->setID($tag_id)
->setName($spec['href'])
->setHref($spec['href'])
->setName($this->assertFlatText($spec['href']))
->setHref($this->assertFlatText($spec['href']))
->setType(PHUITagView::TYPE_OBJECT)
->setExternal(true);
}

View file

@ -180,7 +180,7 @@ final class PhabricatorRemarkupRuleEmbedFile
$autoplay = null;
}
return phutil_tag(
return $this->newTag(
'audio',
array(
'controls' => 'controls',
@ -188,7 +188,7 @@ final class PhabricatorRemarkupRuleEmbedFile
'autoplay' => $autoplay,
'loop' => idx($options, 'loop') ? 'loop' : null,
),
phutil_tag(
$this->newTag(
'source',
array(
'src' => $file->getBestURI(),
@ -203,10 +203,10 @@ final class PhabricatorRemarkupRuleEmbedFile
return id(new PhabricatorFileLinkView())
->setFilePHID($file->getPHID())
->setFileName($options['name'])
->setFileName($this->assertFlatText($options['name']))
->setFileDownloadURI($file->getDownloadURI())
->setFileViewURI($file->getBestURI())
->setFileViewable($options['viewable']);
->setFileViewable((bool)$options['viewable']);
}
private function parseDimension($string) {

View file

@ -3,9 +3,9 @@
final class PhabricatorRemarkupRuleIcon
extends PhutilRemarkupRule {
private $macros;
const KEY_RULE_MACRO = 'rule.macro';
public function getPriority() {
return 50.0;
}
public function apply($text) {
return preg_replace_callback(

View file

@ -147,7 +147,7 @@ final class PhabricatorRemarkupRuleImageMacro
));
}
$result = phutil_tag(
$result = $this->newTag(
'img',
array(
'id' => $id,

View file

@ -8,6 +8,10 @@ final class PhabricatorRemarkupRuleMeme
private $images;
public function getPriority() {
return 50.0;
}
public function apply($text) {
return preg_replace_callback(
'@{meme,((?:[^}\\\\]+|\\\\.)+)}$@m',
@ -42,10 +46,10 @@ final class PhabricatorRemarkupRuleMeme
$options['above'],
$options['below']);
$img = phutil_tag(
$img = $this->newTag(
'img',
array(
'src' => (string)$uri,
'src' => $uri,
'alt' => $alt_text,
));
}

View file

@ -54,7 +54,6 @@ final class PhabricatorPasteRemarkupRule
}
return $embed_paste->render();
return $embed_paste;
}
}

View file

@ -38,7 +38,7 @@ final class PhrictionRemarkupRule
} else if ($this->getEngine()->isTextMode()) {
return PhabricatorEnv::getProductionURI($href);
} else {
$text = phutil_tag(
$text = $this->newTag(
'a',
array(
'href' => $href,

View file

@ -12,6 +12,10 @@ abstract class PhabricatorRemarkupRuleObject
abstract protected function getObjectNamePrefix();
abstract protected function loadObjects(array $ids);
public function getPriority() {
return 50.0;
}
protected function getObjectNamePrefixBeginsWithWordCharacter() {
$prefix = $this->getObjectNamePrefix();
return preg_match('/^\w/', $prefix);
@ -116,11 +120,12 @@ abstract class PhabricatorRemarkupRuleObject
}
// NOTE: The "(?<!#)" prevents us from linking "#abcdef" or similar. The
// "\b" allows us to link "(abcdef)" or similar without linking things
// in the middle of words.
// "(?<!/)" prevents us from linking things in URLs. The "\b" allows us to
// link "(abcdef)" or similar without linking things in the middle of
// words.
$text = preg_replace_callback(
'((?<!#)'.$boundary.$prefix.'('.$id.')(?:#([-\w\d]+))?\b)',
'((?<!#)(?<!/)'.$boundary.$prefix.'('.$id.')(?:#([-\w\d]+))?\b)',
array($this, 'markupObjectReference'),
$text);
@ -212,6 +217,7 @@ abstract class PhabricatorRemarkupRuleObject
$spec['id']);
break;
case 'embed':
$spec['options'] = $this->assertFlatText($spec['options']);
$view = $this->renderObjectEmbed($object, $handle, $spec['options']);
break;
}

View file

@ -30,9 +30,12 @@ final class PhabricatorRemarkupRuleYoutube
}
$youtube_src = 'https://www.youtube.com/embed/'.$v;
$iframe = phutil_tag_div(
'embedded-youtube-video',
phutil_tag(
$iframe = $this->newTag(
'div',
array(
'class' => 'embedded-youtube-video',
),
$this->newTag(
'iframe',
array(
'width' => '650',