Skip to content

Commit

Permalink
Merge pull request #16 from MenschDankeGmbH/2.0
Browse files Browse the repository at this point in the history 
Properly secure output
  • Loading branch information
@torifat
torifat committed Jun 26, 2014
2 parents 547d277 + e007f6e commit fa0a604
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions View/Helper/MenuBuilderHelper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,9 @@ protected function _buildItem(&$item, $pos = -1, &$isActive = false) {
$url = '<a title="' . h($item['title']) . '" href="' . $this->Html->url($item['url']) . '"' . $target . $linkClass . '>';
if (!empty($item['image'])) {
$url .= $this->Html->image($item['image'], array('alt' => $item['title'], 'title' => $item['title']));
$url .= '<span class="label">' . __($item['title']) . '</span>';
$url .= '<span class="label">' . h(__($item['title'])) . '</span>';
} else {
$url .= __($item['title']);
$url .= h(__($item['title']));
}
$url .= '</a>';
}
Expand Down

5 comments on commit fa0a604

@redd
Copy link

@redd redd commented on fa0a604 Jun 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually good to force escaping? Now you can't for eg. use Font Awesome icons in link titles...

@torifat
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redd you can use them using classes I believe.

@redd
Copy link

@redd redd commented on fa0a604 Jun 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not, the markup is for eg. <i class="fa fa-camera-retro"></i> Here goes link title.
I think the best solution should be the same options like in HtmlHelper::link - escape and escapeTitle.

@torifat
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redd ok, I will try to add escaping as an option. Meanwhile if you need then you can use v2.0.1.

@redd
Copy link

@redd redd commented on fa0a604 Jun 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can check my pull request, maybe you'll find something helpful :)

Please sign in to comment.