From 4d3351a5272eebb154f3c4ea4ed606d0182ea11a Mon Sep 17 00:00:00 2001
From: Alexander Purr <purr@ub.uni-leipzig.de>
Date: Mon, 25 Oct 2021 14:58:50 +0200
Subject: [PATCH] refs #19650 [finc] external link view helper

* remove translator (didn't work anyway)
* no url escape
* manual escape check for attributes and label
* using external link view helper in finc templates
---
 .../finc/View/Helper/Root/ExternalLink.php    | 59 +++++++++++++------
 .../templates/Recommend/EbscoResults.phtml    | 16 ++---
 .../templates/RecordTab/acquisitionpda.phtml  | 10 +++-
 .../templates/RecordTab/holdingsils.phtml     |  6 +-
 .../finc/templates/ajax/resolverLinks.phtml   |  7 ++-
 themes/finc/templates/ajax/status-full.phtml  |  2 +-
 themes/finc/templates/amsl/sources-list.phtml | 16 +++--
 themes/finc/templates/footer.phtml            |  4 +-
 themes/finc/templates/record/pdaform.phtml    | 10 +++-
 9 files changed, 88 insertions(+), 42 deletions(-)

diff --git a/module/finc/src/finc/View/Helper/Root/ExternalLink.php b/module/finc/src/finc/View/Helper/Root/ExternalLink.php
index 91b1df9feaa..6bfbb6de3cc 100644
--- a/module/finc/src/finc/View/Helper/Root/ExternalLink.php
+++ b/module/finc/src/finc/View/Helper/Root/ExternalLink.php
@@ -27,6 +27,9 @@
  */
 namespace finc\View\Helper\Root;
 
+use Zend\View\Helper\EscapeHtml;
+use Zend\View\Helper\EscapeHtmlAttr;
+
 /**
  * External link view helper
  *
@@ -38,13 +41,6 @@ namespace finc\View\Helper\Root;
  */
 class ExternalLink extends \Zend\View\Helper\AbstractHelper
 {
-    /**
-     * Context view helper
-     *
-     * @var \VuFind\View\Helper\Root\Translate
-     */
-    protected $translator;
-
     /**
      * Default html attributes for external links
      *
@@ -55,6 +51,13 @@ class ExternalLink extends \Zend\View\Helper\AbstractHelper
         'rel' => 'noopener'
     ];
 
+    /**
+     * Escape manuelly flag
+     *
+     * @var bool
+     */
+    protected $manualEscape;
+
     /**
      * Renders an anchor element (hyperlink) to an external website
      *
@@ -62,7 +65,8 @@ class ExternalLink extends \Zend\View\Helper\AbstractHelper
      * @param string $label          desired label or translation token
      * @param array  $attributes     more attributes to apply to the a element,
      *                               key-value-pairs
-     * @param bool   $translateLabel true if label shall be translated
+     * @param bool   $manualEscape   true if $label conatins html to display and
+     *                               $attributes are escped before calling method
      *
      * @return string
      */
@@ -70,36 +74,55 @@ class ExternalLink extends \Zend\View\Helper\AbstractHelper
         $href,
         $label = '',
         $attributes = [],
-        $translateLabel = false
+        $manualEscape = false
     ) {
+        $this->manualEscape = $manualEscape;
+
         $link = '<a href="' . $href . '"';
         $attributes = array_merge($this->defaultAttributes, $attributes);
         foreach ($attributes as $key => $value) {
-            $link .= ' ' . $key . '="' . $value . '"';
+            $link .= ' ' . $key . '="' . $this->escapeHtmlAttr($value) . '"';
         }
         $link .= '>';
         if (empty($label)) {
             $label = $href;
-        } elseif ($translateLabel) {
-            $label = $this->translate($label);
         }
-        $link .= $label . '</a>';
+        $link .= $this->escapeHtml($label) . '</a>';
 
         return $link;
     }
 
     /**
-     * Helper function to provide access to Translate ViewHelper
+     * Helper function to provide access to EscapeHTML ViewHelper
+     * and esacpe lable if not disabled
+     *
+     * @param string $html html to escape
+     *
+     * @return string
+     */
+    protected function escapeHtml($html)
+    {
+        if ($this->manualEscape) {
+            return $html;
+        }
+        $htmlEscaper = $this->getView()->plugin('escapeHtml');
+        return $htmlEscaper($html);
+    }
+
+    /**
+     * Helper function to provide access to EscaperHTML Attribute ViewHelper
+     * and esacpe HTML attributes if not disabled
      *
      * @param string $label translation token
      *
      * @return string
      */
-    protected function translate($label)
+    protected function escapeHtmlAttr($htmlAttr)
     {
-        if (!isset($this->translator)) {
-            $this->translator = $this->getView()->plugin('translate');
+        if ($this->manualEscape) {
+            return $htmlAttr;
         }
-        return $this->translator->translate($label);
+        $htmlAttrEscaper = $this->getView()->plugin('escapeHtmlAttr');
+        return $htmlAttrEscaper($htmlAttr);
     }
 }
diff --git a/themes/finc/templates/Recommend/EbscoResults.phtml b/themes/finc/templates/Recommend/EbscoResults.phtml
index 2801644566d..253b18944ff 100644
--- a/themes/finc/templates/Recommend/EbscoResults.phtml
+++ b/themes/finc/templates/Recommend/EbscoResults.phtml
@@ -13,15 +13,17 @@ if (is_array($data['results']) && count($data['results']) > 0): ?>
         <span class="badge">
                 <?= $this->escapeHtml($result['hits']) ?>
             </span>
-            <a href="<?= $this->escapeHtmlAttr($result['url']) ?>" target="_blank">
-                <?= $this->escapeHtml($this->truncate($this->transEsc('Ebsco::'.$result['database']), 90)) ?>
-            </a>
+        <?= $this->externalLink(
+              $this->escapeHtmlAttr($result['url']),
+              $this->truncate($this->translate('Ebsco::'.$result['database']), 90)
+            ) ?>
       </div>
         <?php endforeach; ?>
-        <a class="facet narrow-toggle" href="<?=$this->escapeHtmlAttr($data['hits_total_url'])?>" target="_blank" rel="nofollow">
-          <?=$this->transEsc('more')?>&nbsp;...
-            </a>
-
+      <?= $this->externalLink(
+            $data['hits_total_url'],
+            $this->translate('more'),
+            ['class' => 'facet narrow-toggle']
+          ) ?>
       </div>
   </div>
 <?php endif; ?>
diff --git a/themes/finc/templates/RecordTab/acquisitionpda.phtml b/themes/finc/templates/RecordTab/acquisitionpda.phtml
index 91296ba15fa..e069017a899 100644
--- a/themes/finc/templates/RecordTab/acquisitionpda.phtml
+++ b/themes/finc/templates/RecordTab/acquisitionpda.phtml
@@ -15,9 +15,13 @@ $controllerClass = 'controller:SolrMarcFincPDA';
 <p class="alert alert-info"><?=$this->transEsc('PDA::pda_restriction_text')?></p>
 <div class="btn-group">
   <?php /* Leave title in here - it is used for the tooltip! - CK */ ?>
-  <a class="btn btn-primary" data-toggle="tooltip" title="<?=$this->transEsc('PDA::pda_open_new_window')?>" href="<?=$this->interlibraryloan()->getSwbLink($this->driver)?>" target="_blank">
-    <?=$this->transEsc('PDA::pda_tab_interlibrary_button')?>
-  </a>
+  <?= $this->externalLink(
+        $this->interlibraryloan()->getSwbLink($this->driver),
+        $this->translate('PDA::pda_tab_interlibrary_button'),
+        [ 'class' => 'btn btn-primary',
+          'data-toggle' => 'tooltip',
+          'title' => $this->translate('PDA::pda_open_new_window')]
+      ) ?>
   <a class="btn btn-primary pda-button <?=$controllerClass?>" data-lightbox href="<?=$this->url('record-pda', array('id' => $id))?>" rel="nofollow">
     <?=$this->transEsc('PDA::pda_tab_order_button')?>
   </a>
diff --git a/themes/finc/templates/RecordTab/holdingsils.phtml b/themes/finc/templates/RecordTab/holdingsils.phtml
index d929e6b59b9..ed99f581cf8 100644
--- a/themes/finc/templates/RecordTab/holdingsils.phtml
+++ b/themes/finc/templates/RecordTab/holdingsils.phtml
@@ -65,7 +65,7 @@ if (!empty($holdingTitleHold)): ?>
   <h2><?=$this->transEsc("Internet")?></h2>
   <?php if (!empty($urls)): ?>
     <?php foreach ($urls as $current): ?>
-      <a href="<?=$this->escapeHtmlAttr($this->proxyUrl($current['url']))?>"><?=$this->escapeHtml($current['desc'])?></a><br/>
+      <?= $this->externalLink($this->escapeHtmlAttr($this->proxyUrl($current['url'])), $current['desc']) ?><br/>
     <?php endforeach; ?>
   <?php endif; ?>
   <?php /* finc-specific snippet - #9274 - replaces if ($openUrlActive): ... - CK */ ?>
@@ -75,7 +75,7 @@ if (!empty($holdingTitleHold)): ?>
       if (!empty($fallbackUrls)): ?>
         <span id="urlsHideable" style="display: none">
           <?php foreach ($fallbackUrls as $current): ?>
-            <a href="<?=$this->escapeHtmlAttr($this->proxyUrl($current['url']))?>" target="_blank"><?=$this->escapeHtml($current['desc'] ?? $current['url'])?></a><br/>
+            <?= $this->externalLink($this->escapeHtmlAttr($this->proxyUrl($current['url'])), $current['desc'] ?? $current['url']) ?><br/>
           <?php endforeach; ?>
         </span>
       <?php endif; ?>
@@ -93,7 +93,7 @@ if (!empty($holdingTitleHold)): ?>
   <h2>
     <?php $locationText = $this->transEsc('location_' . $holding['location'], [], $holding['location']); ?>
     <?php if (isset($holding['locationhref']) && $holding['locationhref']): ?>
-      <a href="<?=$holding['locationhref']?>" target="_blank"><?=$locationText?></a>
+      <?= $this->externalLink($holding['locationhref'], $locationText) ?>
     <?php else: ?>
       <?=$locationText?>
     <?php endif; ?>
diff --git a/themes/finc/templates/ajax/resolverLinks.phtml b/themes/finc/templates/ajax/resolverLinks.phtml
index d9b82875c17..cdc5ac85fac 100644
--- a/themes/finc/templates/ajax/resolverLinks.phtml
+++ b/themes/finc/templates/ajax/resolverLinks.phtml
@@ -22,7 +22,12 @@
               <?php /* finc-specific change #7986 - END */ ?>
               <a href="<?=$this->escapeHtmlAttr($link['href'])?>" title="<?=isset($link['service_type'])?$this->escapeHtmlAttr($link['service_type']):''?>"<?=!empty($link['access'])?' class="access-'.$link['access'].'"':''?>><?=isset($link['title'])?$this->escapeHtml($link['title']):''?></a> <br />
               <?php /* finc-specific change #5334 - CK */ ?>
-              <small><?=isset($link['coverage'])?$this->escapeHtml($link['coverage']):''?><?=isset($link['coverageHref'])?' <a href="'.$link['coverageHref'].'" target="_blank">'.$this->translate('Readme').'</a>':''?></small>
+              <small>
+                <?= isset($link['coverage']) ? $this->escapeHtml($link['coverage']) : '' ?>
+                <?= isset($link['coverageHref'])
+                      ? $this->externalLink($link['coverageHref'], $this->translate('Readme'))
+                      : '' ?>
+              </small>
               <?php /* finc-specific change #5334 - END */ ?>
             <?php else: ?>
               <?=isset($link['title'])?$this->escapeHtml($link['title']):''?> <?=isset($link['coverage'])?$this->transEsc($link['coverage']):''?>
diff --git a/themes/finc/templates/ajax/status-full.phtml b/themes/finc/templates/ajax/status-full.phtml
index 303067afd3f..f48411b6a76 100644
--- a/themes/finc/templates/ajax/status-full.phtml
+++ b/themes/finc/templates/ajax/status-full.phtml
@@ -12,7 +12,7 @@
       <td data-title="<?= $this->transEsc('Location') ?>:" class="fullLocation">
         <?php $locationText = $this->transEsc('location_' . $item['location'], [], $item['location']); ?>
         <?php if (isset($item['locationhref']) && $item['locationhref']): ?>
-          <a href="<?=$item['locationhref']?>" target="_blank"><?=$locationText?></a>
+          <?= $this->externalLink($item['locationhref'], $locationText) ?>
         <?php else: ?>
           <?=$locationText?>
         <?php endif; ?>
diff --git a/themes/finc/templates/amsl/sources-list.phtml b/themes/finc/templates/amsl/sources-list.phtml
index 371f906684d..8970d0d041d 100644
--- a/themes/finc/templates/amsl/sources-list.phtml
+++ b/themes/finc/templates/amsl/sources-list.phtml
@@ -49,9 +49,11 @@ $this->layout()->breadcrumbs .= '</li> <li class="active">' . $this->transEsc('L
             <?php foreach ($source as $sub_label => $collection): ?>
               <li>
                 <?php if (!empty($collection['href'])): ?>
-                  <a title="<?=$this->transEsc("Search For")?> <?=$sub_label?>" href='<?=$collection["href"]?>' target="_blank">
-                    <?=$sub_label?>
-                  </a>
+                <?= $this->externalLink(
+                      $collection["href"],
+                      $sub_label,
+                      ['title' => "{$this->transEsc("Search For")} {$sub_label}"]
+                    ) ?>
                 <?php else: ?>
                   <div tabindex="0" aria-label="<?=$this->transEsc("Source Title")?>">
                       <?=$sub_label?>
@@ -77,8 +79,12 @@ $this->layout()->breadcrumbs .= '</li> <li class="active">' . $this->transEsc('L
       <span>
         <?=$this->transEsc('support_by_dfg');?>
       </span>
-      <a href='http://www.dfg.de' target='_blank'>
-        <img src='<?=$this->imageLink('dfg_logo_text.png')?>' alt='Deutsche Forschungsgemeinschaft, DFG'>
+      <?= $this->externalLink(
+            'http://www.dfg.de',
+            "<img src='{$this->imageLink('dfg_logo_text.png')}' alt='Deutsche Forschungsgemeinschaft, DFG'>",
+            [],
+            true
+          ) ?>
       </a>
     </div>
   </div>
diff --git a/themes/finc/templates/footer.phtml b/themes/finc/templates/footer.phtml
index 301b47e88a4..719d70099b3 100644
--- a/themes/finc/templates/footer.phtml
+++ b/themes/finc/templates/footer.phtml
@@ -37,8 +37,8 @@
         <span>
           <?= $this->transEsc("Footer-Powered-By-Text") ?>
         </span>
-        <?= $this->externalLink("https://vufind.org", '<img src="' . $this->imageLink('vufind_logo.png') . '" alt="' . $this->translate('vufind-logo_alt') . '" aria-hidden="true"/>', ['title' => $this->translate('vufind-logo_title')]) ?>
-        <?= $this->externalLink("https://finc.info", '<img src="' . $this->imageLink('finc_logo.png') . '" alt="' . $this->translate('finc-logo_alt') . '" aria-hidden="true"/>', ['title' => $this->translate('finc-logo_title')]) ?>
+        <?= $this->externalLink("https://vufind.org", '<img src="' . $this->imageLink('vufind_logo.png') . '" alt="' . $this->translate('vufind-logo_alt') . '" aria-hidden="true"/>', ['title' => $this->translate('vufind-logo_title')], true) ?>
+        <?= $this->externalLink("https://finc.info", '<img src="' . $this->imageLink('finc_logo.png') . '" alt="' . $this->translate('finc-logo_alt') . '" aria-hidden="true"/>', ['title' => $this->translate('finc-logo_title')], true) ?>
       </div>
     </div>
 </footer>
diff --git a/themes/finc/templates/record/pdaform.phtml b/themes/finc/templates/record/pdaform.phtml
index a7faa1f04c5..ab7cb443865 100644
--- a/themes/finc/templates/record/pdaform.phtml
+++ b/themes/finc/templates/record/pdaform.phtml
@@ -36,8 +36,14 @@ $this->layout()->breadcrumbs = '<li>' . $this->searchMemory()->getLastSearchLink
       <?=$this->recaptcha()->html($this->useRecaptcha)?>
     </span>
     <input type="submit" class="btn btn-primary" role="button" name="submit" value="<?=$this->transEsc('Submit')?>"/>
-    <a class="btn btn-primary" data-lightbox-ignore data-toggle="tooltip" title="<?=$this->transEsc('PDA::pda_open_new_window')?>" href="<?=$this->interlibraryloan()->getSwbLink($this->driver)?>" target="_blank"><?=$this->transEsc('PDA::pda_tab_interlibrary_button')?>
-    </a>
+    <?= $this->externalLink(
+          $this->interlibraryloan()->getSwbLink($this->driver),
+          $this->translate('PDA::pda_tab_interlibrary_button'),
+          ['class' => 'btn btn-primary',
+           'data-lightbox-ignore' => '',
+           'data-toggle' => 'tooltip',
+           'title' => $this->translate('PDA::pda_open_new_window')]
+         ) ?>
     <button class="btn btn-transparent" type="button" data-dismiss="modal" href="#"><?=$this->transEsc('Reset')?></button>
   </div>
 
-- 
GitLab