From fcdaa0f0610a14c20254c4933d891fb333a62ecc Mon Sep 17 00:00:00 2001
From: Carsten Rose <carsten.rose@math.uzh.ch>
Date: Sun, 7 Mar 2021 22:58:46 +0100
Subject: [PATCH] Implements #12085. Add doc and custom error message. Finalize
directDownload notation. Add unit test.
---
Documentation/Report.rst | 86 ++++++++++++++++---
extension/Classes/Core/Report/Download.php | 30 +++++--
extension/Classes/Core/Report/Link.php | 60 ++++++++-----
extension/Classes/Core/Store/Session.php | 1 +
extension/Tests/Unit/Core/Report/LinkTest.php | 36 +++++++-
extension/ext_conf_template.txt | 13 +++
6 files changed, 185 insertions(+), 41 deletions(-)
diff --git a/Documentation/Report.rst b/Documentation/Report.rst
index 41d21c895..56c2fc3b8 100644
--- a/Documentation/Report.rst
+++ b/Documentation/Report.rst
@@ -1945,20 +1945,45 @@ Output::
Download
--------
-Download offers:
++--------------------+--------------------------------------------------------+----------------------------------------+
+| Mode | Security | Note |
++====================+========================================================+========================================+
+| Direct File access | Files are public available. No access restriction | Use ``<a href="...">`` |
+| | | Pro: Simple, links can be copied. | | Merge multiple sources: no |
+| | | Con: Directory access, guess of filenames, only | but check :ref:`column-save-pdf` |
+| | removing the file will deny access. | | Custom 'save as filename': no |
++--------------------+--------------------------------------------------------+----------------------------------------+
+| Persistent Link | Access is be defined by a SQL statement. In *T3/BE | Use ``..., 'd:1234|s:0' AS _link`` |
+| | > Extension > QFQ > File > download* define a SQL | | Merge multiple sources: yes |
+| | statement. | | Custom 'save as filename': yes |
+| | | Pro: speaking URL, link can be copied, access can | |
+| | can be defined a SQL statement. | |
+| | | Con: **Key might be altered by user**, permission | |
+| | can't be user logged in dependent. | |
++--------------------+--------------------------------------------------------+----------------------------------------+
+| Secure Link | **Default**. SIP protected link. | Use ``..., 'd|F:file.pdf' AS _link`` |
+| | | Pro: Parameter can't be altered, most easy definition| | Merge multiple sources: yes |
+| | in QFQ, access might be logged in user dependent. | | Custom 'save as filename': yes |
+| | | Cons: Links are assigned to a browser session and | |
+| | can't be copied | |
++--------------------+--------------------------------------------------------+----------------------------------------+
+
+The rest of this section applies only to `Persistent Link` and `Secure Link`. Download offers:
* Single file - download a single file (any type),
* PDF create - one or concatenate several files (uploaded) and/or web pages (=HTML to PDF) into one PDF output file,
* ZIP archive - filled with several files ('uploaded' or 'HTML to PDF'-converted).
* Excel - created from scratch or fill a template xlsx with database values.
-The downloads are SIP protected. Only the current user can use the link to download files.
-
By using the `_link` column name:
-* the option `d:...` initiate creating the download link and optional specifies an export filename,
+* the option `d:...` initiate creating the download link and optional specifies
+
+ * in `SIP` mode: an export filename (),
+ * in `persistent link` mode: path download script (optional) and key(s) to identify the record with the PathFilename
+ information (see below).
+
* the optional `M:...` (Mode) specifies the export type (file, pdf, zip, export),
-* setting `s:1` is recommended for the download function (file / path name is hidden to the user),
* the alttext `a:...` specifies a message in the download popup.
By using `_pdf`, `_Pdf`, `_file`, `_File`, `_zip`, `_Zip`, `_excel` as column name, the options `d`, `M` and `s`
@@ -1967,16 +1992,12 @@ will be set.
All files will be read by PHP - therefore the directory might be protected against direct web access. This is the
preferred option to offer secure downloads via QFQ.
-In case the download needs a persistant URL (no SIP, no user session), a regular
-link, pointing directly to a file, have to be used - the download functionality described here is not appropriate for
-such a scenario. If necessary, :ref:`column-save-pdf` can be used to generate such a file.
-
.. _download-parameter-files:
Parameter and (element) sources
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-* *download*: `d[:<exportFilename>]`
+* mode `secure link` (s:1) - *download*: `d[:<exportFilename>]`
* *exportFilename* = <filename for save as> - Name, offered in the 'File save as' browser dialog. Default: 'output.<ext>'.
@@ -1984,6 +2005,51 @@ Parameter and (element) sources
The user typically expects meaningful and distinct file names for different download links.
+* mode `persistent link` (s:0) - *download*: `d:[<path/name>]<key1>[/<keyN>]`
+
+ This setup is divided in part a) and b):
+
+ Part a) - offering the download link.
+
+ * The whole part a) is optional. The download itself will work without it.
+
+ * (Optional) *path/name* = of the QFQ `download.php` script. By default``typo3conf/ext/qfq/Classes/Api/download.php``.
+ Three further possibilities: ``dl.php`` or ``dl2.php`` or ``dl3.php`` (see below).
+
+ * *key1* = give a uniq identifier to select the wished record.
+
+ Part b) - process the download
+
+ * In the QFQ extension config: File > Query for direct download mode: `download.php` or `dl.php` or `dl2.php` or `dl3.php`
+ up to 4 different SQL statements can be given with the regular QFQ download link syntax (skip the visual elements
+ like button, text, glyph icon, question,...)::
+
+ SELECT CONCAT('d|F:', n.pathFileName) FROM Note AS n WHERE n.id=?
+
+ All `?` in the SQL statement will be replaced by the specified parameter. If there are more `?` than parameter,
+ the last parameter will be reused for all pending `?`.
+
+ E.g. ``10.sql = SELECT 'd:1234|t:File.pdf' AS _link`` creates a link
+ ``<a href="typo3conf/ext/qfq/Classes/Api/download.php/1234"><span class="btn btn-default">File.pdf</span></span>``.
+ If the user clicks on the link, QFQ will extract the `1234` argument and via ``download.php`` the query (defined in
+ the Typo QFQ extension config) will be prepared and fires ``SELECT CONCAT('d|F:', n.pathFileName, '|t:File.pdf') FROM Note AS n WHERE n.id=1234``.
+ The download of the file, specified by ``n.pathFileName``, will start.
+
+ If no record ist selected, a custom error will be shown. If the query selectes more than one record, a general error will be shown.
+
+ If one of ``dl.php`` or ``dl2.php`` or ``dl3.php`` should be used, please initially create the symlink(s), e.g. in the
+ application directory (same level as typo3conf) ``ln -s typo3conf/ext/qfq/Classes/Api/download.php dl.php`` (or dl2.ph, dl3.php).
+
+ Speaking URL)
+
+ Instead of using a numeric value reference key, also a text can be used. Always take care that exactly one record is
+ selected. The key is transferred by URL therefore untrusted: The sanitize class :ref:`alnumx<sanitize-class>` is applied.
+ Example::
+
+ Query: SELECT CONCAT('d|F:', n.pathFileName) FROM Person AS p WHERE p.name=? AND p.firstName=? AND p.publish='yes'
+ Link: https://example.com/dl.php/doe/john
+
+
* *popupMessage*: `a:<text>` - will be displayed in the popup window during download. If the creating/download is fast, the window might disappear quickly.
* *mode*: `M:<mode>`
diff --git a/extension/Classes/Core/Report/Download.php b/extension/Classes/Core/Report/Download.php
index a830a6940..0f88d7515 100644
--- a/extension/Classes/Core/Report/Download.php
+++ b/extension/Classes/Core/Report/Download.php
@@ -684,7 +684,7 @@ class Download {
* Retrieve SQL query from QFQ config, specific to script name.
* Four script names are possible: download.php, dl.php, dl2.php, dl3.php
*
- * @return array|bool|int|string|null
+ * @return array // [ 'sql' -> 'SELECT "d|F:file.pdf"', 'error' -> 'Record not found']
* @throws \CodeException
* @throws \DbException
* @throws \UserFormException
@@ -696,7 +696,16 @@ class Download {
// Example: /var/www/html/qfq/dl.php >> dl.php
$scriptName = substr($scriptName, strrpos($scriptName, '/') + 1);
- return $this->store->getVar(SYSTEM_SQL_DIRECT_DOWNLOAD . $scriptName, STORE_SYSTEM . STORE_EMPTY);
+ $arr['sql'] = $this->store->getVar(SYSTEM_SQL_DIRECT_DOWNLOAD . $scriptName, STORE_SYSTEM . STORE_EMPTY);
+ if ($arr['sql'] == '') {
+ throw new \DownloadException(json_encode([ERROR_MESSAGE_TO_USER => 'Missing SQL',
+ ERROR_MESSAGE_TO_DEVELOPER => "Missing SQL in QFQ extension config variable: " . SYSTEM_SQL_DIRECT_DOWNLOAD . $scriptName
+ ])
+ , ERROR_MISSING_VALUE);
+ }
+
+ $arr['error'] = $this->store->getVar(SYSTEM_SQL_DIRECT_DOWNLOAD . $scriptName . 'error', STORE_SYSTEM . STORE_EMPTY);
+ return $arr;
}
/**
@@ -707,10 +716,10 @@ class Download {
* @throws \UserReportException
*/
private function getDirectDownloadModeDetails() {
- //TODO was ist wenn sql leer ist
- $sql = $this->getDirectDownloadSql();
- // Get, Clean: with http://loclhost/qfq/typo3conf/ext/qfq/Classes/Api/download.php/help is $_SERVER['PATH_INFO']='/help'.
+ $arr = $this->getDirectDownloadSql();
+
+ // Get, Clean: with http://localhost/qfq/typo3conf/ext/qfq/Classes/Api/download.php/help is $_SERVER['PATH_INFO']='/help'.
$pathInfo = $this->store->getVar('PATH_INFO', STORE_CLIENT . STORE_EMPTY, SANITIZE_ALLOW_ALNUMX);
$pathInfo = Sanitize::sanitize(urldecode($pathInfo), SANITIZE_ALLOW_ALNUMX);
@@ -718,14 +727,17 @@ class Download {
// In case there are more question mark than parameter, duplicate the last parameter until enough parameter filled.
$last = end($param);
- $questionMark = substr_count($sql, '?');
+ $questionMark = substr_count($arr['sql'], '?');
while ($questionMark > count($param)) {
$param[] = $last;
}
-//TODO SQL Fehler abfangen: SQL Fehler ausgeben!!
+
+ if ($arr['error'] == '') {
+ $arr['error'] = 'Key for download not found.';
+ }
// Get cmd which defines the download
- $param = $this->db->sql($sql, ROW_EXPECT_1, $param);
- // In case there are more than on column: implode
+ $param = $this->db->sql($arr['sql'], ROW_EXPECT_1, $param, $arr['error']);
+ // In case there are more than one column: implode
$cmd = implode('', $param);
// Use the link class only to reuse the parsing code of the download element.
$link = new Link(Store::getSipInstance());
diff --git a/extension/Classes/Core/Report/Link.php b/extension/Classes/Core/Report/Link.php
index 6150aebb2..013925d89 100644
--- a/extension/Classes/Core/Report/Link.php
+++ b/extension/Classes/Core/Report/Link.php
@@ -850,12 +850,12 @@ class Link {
}
// Download Link needs some extra work
- if (isset($rcTokenGiven[TOKEN_DOWNLOAD]) && $rcTokenGiven[TOKEN_DOWNLOAD]) {
+ if ($rcTokenGiven[TOKEN_DOWNLOAD] ?? false) {
$vars = $this->buildDownloadLate($vars);
}
// CopyToClipboard (Download) Link needs some extra work
- if (isset($rcTokenGiven[TOKEN_COPY_TO_CLIPBOARD]) && $rcTokenGiven[TOKEN_COPY_TO_CLIPBOARD]) {
+ if ($rcTokenGiven[TOKEN_COPY_TO_CLIPBOARD] ?? false) {
$vars = $this->buildCopyToClipboardLate($vars);
}
@@ -985,11 +985,6 @@ class Link {
case LINK_PICTURE:
$countLinkPicture++;
break;
- case NAME_FILE:
- case NAME_URL:
- case NAME_PAGE:
- $countSources++;
- break;
default:
break;
}
@@ -1008,10 +1003,14 @@ class Link {
throw new \UserReportException ("Token Mail and Target at the same time not possible'" . TOKEN_PAGE . "'", ERROR_MULTIPLE_DEFINITION);
}
- if (isset($tokenGiven[TOKEN_DOWNLOAD]) && count($vars[NAME_COLLECT_ELEMENTS]) == 0 && $countSources == 0) {
+ if (isset($tokenGiven[TOKEN_DOWNLOAD]) && $vars[NAME_SIP] == "1" && count($vars[NAME_COLLECT_ELEMENTS]) == 0) {
throw new \UserReportException ("Missing element sources for download", ERROR_MISSING_REQUIRED_PARAMETER);
}
+ if (isset($tokenGiven[TOKEN_DOWNLOAD]) && $vars[NAME_SIP] == "0" && count($vars[NAME_COLLECT_ELEMENTS]) > 0) {
+ throw new \UserReportException ("For persistent downloads, sources are not necessary/allowed in the link definition. Instead, please define the sources in QFQ extension setup > file.", ERROR_MISSING_REQUIRED_PARAMETER);
+ }
+
if (isset($tokenGiven[TOKEN_ACTION_DELETE])) {
$this->checkDeleteParam($vars);
}
@@ -1107,11 +1106,14 @@ class Link {
$vars[NAME_EXTRA_CONTENT_WRAP] = str_replace(DOWNLOAD_POPUP_REPLACE_TEXT, $altText, $vars[NAME_EXTRA_CONTENT_WRAP]);
$vars[NAME_EXTRA_CONTENT_WRAP] = str_replace(DOWNLOAD_POPUP_REPLACE_TITLE, 'Download: ' . addslashes($vars[NAME_DOWNLOAD]), $vars[NAME_EXTRA_CONTENT_WRAP]);
- $tmpUrlParam = array();
- $tmpUrlParam[DOWNLOAD_MODE] = $this->getDownloadModeNCheck($vars);
- $tmpUrlParam[DOWNLOAD_EXPORT_FILENAME] = $vars[NAME_DOWNLOAD];
- $tmpUrlParam[SIP_DOWNLOAD_PARAMETER] = base64_encode(implode(PARAM_DELIMITER, $vars[NAME_COLLECT_ELEMENTS]));
- $vars[NAME_URL_PARAM] = KeyValueStringParser::unparse($tmpUrlParam, '=', '&');
+ if ($vars[NAME_SIP] == '1' || $vars[NAME_SIP] == '') {
+ // Special encoding only necessary for SIP based downloads.
+ $tmpUrlParam = array();
+ $tmpUrlParam[DOWNLOAD_MODE] = $this->getDownloadModeNCheck($vars);
+ $tmpUrlParam[DOWNLOAD_EXPORT_FILENAME] = $vars[NAME_DOWNLOAD];
+ $tmpUrlParam[SIP_DOWNLOAD_PARAMETER] = base64_encode(implode(PARAM_DELIMITER, $vars[NAME_COLLECT_ELEMENTS]));
+ $vars[NAME_URL_PARAM] = KeyValueStringParser::unparse($tmpUrlParam, '=', '&');
+ }
}
// CopyToClipboard
@@ -1655,6 +1657,8 @@ EOF;
* @param $vars
* @return array
* @throws \CodeException
+ * @throws \UserFormException
+ * @throws \UserReportException
*/
private function buildDownloadLate($vars) {
@@ -1683,6 +1687,11 @@ EOF;
}
}
+ // Download links should be by default SIP encoded.
+ if (($vars[NAME_SIP]) === false) {
+ $vars[NAME_SIP] = "1";
+ }
+
// No download link! Only text/button
if ($vars[NAME_RENDER] == '3') {
return $vars;
@@ -1703,18 +1712,29 @@ EOF;
$onClick = <<<EOF
onclick="$('#qfqModalTitle101').text($(this).data('title')); $('#qfqModalText101').text($(this).data('text'));"
EOF;
-
- $vars[NAME_URL] = Path::appToApi(API_DOWNLOAD_PHP);
+ // Check for SIP encoded download link or persistent download link
+ if ($vars[NAME_SIP] == '1') {
+ // SIP encoded.
+ $vars[NAME_URL] = Path::appToApi(API_DOWNLOAD_PHP);
+ } else {
+ // Persistent Download Link
+ if ($vars[NAME_DOWNLOAD] == '') {
+ throw new \UserReportException("Persistent download link (s:0): please specify target key like 'd:1234'", ERROR_MISSING_VALUE);
+ }
+ // Check if there is a shortcut version defined like d:dl.php/123.
+ if (strstr($vars[NAME_DOWNLOAD], '.php/') === false) {
+ // Set the default: API_DOWNLOAD_PHP
+ $vars[NAME_URL] = Path::appToApi(API_DOWNLOAD_PHP) . '/' . $vars[NAME_DOWNLOAD];
+ } else {
+ // Take given shortcut
+ $vars[NAME_URL] = $vars[NAME_DOWNLOAD];
+ }
+ }
$vars[NAME_LINK_CLASS_DEFAULT] = NO_CLASS;
$vars[NAME_EXTRA_CONTENT_WRAP] = '<span ' . $attributes . $onClick . '>';
$vars[NAME_BOOTSTRAP_BUTTON] = '0';
- // Download links should be by default SIP encoded.
- if (($vars[NAME_SIP]) === false) {
- $vars[NAME_SIP] = "1";
- }
-
return $vars;
}
diff --git a/extension/Classes/Core/Store/Session.php b/extension/Classes/Core/Store/Session.php
index e444afe5f..c27a38199 100644
--- a/extension/Classes/Core/Store/Session.php
+++ b/extension/Classes/Core/Store/Session.php
@@ -44,6 +44,7 @@ class Session
if (self::$phpUnit === true) {
self::$sessionLocal = array();
} else {
+
ini_set('session.cookie_httponly', 1);
$lifetime = SYSTEM_COOKIE_LIFETIME;
diff --git a/extension/Tests/Unit/Core/Report/LinkTest.php b/extension/Tests/Unit/Core/Report/LinkTest.php
index 523ec6fa3..d30cdaaa1 100644
--- a/extension/Tests/Unit/Core/Report/LinkTest.php
+++ b/extension/Tests/Unit/Core/Report/LinkTest.php
@@ -1506,17 +1506,30 @@ EOF;
$result = $link->renderLink('d');
}
+ /**
+ * @expectedException UserReportException
+ *
+ * @throws \CodeException
+ * @throws \UserFormException
+ * @throws \UserReportException
+ */
+ public function testDownloadException1() {
+ $link = new Link($this->sip, DB_INDEX_DEFAULT, true);
+
+ // Empty
+ $result = $link->renderLink('d|F:file.pdf|F:file2.pdf|s:0');
+ }
+
/**
* @throws \CodeException
* @throws \UserFormException
* @throws \UserReportException
*/
- public function testDownload() {
+ public function testDownloadSecureLink() {
$link = new Link($this->sip, DB_INDEX_DEFAULT, true);
// Single file
$result = $link->renderLink('d|F:file.pdf');
-// $this->assertEquals('<a href="typo3conf/ext/qfq/Classes/Api/download.php?mode=pdf&_exportFilename=&_b64_download=RjpmaWxlLnBkZg==" class="0" title="Download" ><span class="btn btn-default" data-toggle="modal" data-target="#qfqModal101" data-title="Download: " data-text="Please wait" data-backdrop="static" data-keyboard="false" onclick="$(\'#qfqModalTitle101\').text($(this).data(\'title\')); $(\'#qfqModalText101\').text($(this).data(\'text\'));"><span class="glyphicon glyphicon-file" ></span></span></a>', $result);
$this->assertEquals('<a href="typo3conf/ext/qfq/Classes/Api/download.php?s=badcaffee1234" class="0" title="Download" ><span class="btn btn-default" data-toggle="modal" data-target="#qfqModal101" data-title="Download: " data-text="Please wait" data-backdrop="static" data-keyboard="false" onclick="$(\'#qfqModalTitle101\').text($(this).data(\'title\')); $(\'#qfqModalText101\').text($(this).data(\'text\'));"><span class="glyphicon glyphicon-file" ></span></span></a>', $result);
// With download filename
@@ -1577,6 +1590,25 @@ EOF;
}
+ /**
+ * @throws \CodeException
+ * @throws \UserFormException
+ * @throws \UserReportException
+ */
+ public function testDownloadPersistentLink() {
+ $link = new Link($this->sip, DB_INDEX_DEFAULT, true);
+
+ Store::setVar(SYSTEM_SQL_DIRECT_DOWNLOAD . 'downloadphp', 'SELECT "d|F:file.pdf"', STORE_SYSTEM);
+
+ $result = $link->renderLink('d:123|s:0');
+ $this->assertEquals('<a href="typo3conf/ext/qfq/Classes/Api/download.php/123" class="0" title="Download" ><span class="btn btn-default" data-toggle="modal" data-target="#qfqModal101" data-title="Download: 123" data-text="Please wait" data-backdrop="static" data-keyboard="false" onclick="$(\'#qfqModalTitle101\').text($(this).data(\'title\')); $(\'#qfqModalText101\').text($(this).data(\'text\'));"><span class="glyphicon glyphicon-file" ></span></span></a>', $result);
+
+ Store::setVar(SYSTEM_SQL_DIRECT_DOWNLOAD . 'dlphp', 'SELECT "d|F:file.pdf"', STORE_SYSTEM);
+ $result = $link->renderLink('d:dl.php/123|s:0');
+ $this->assertEquals('<a href="dl.php/123" class="0" title="Download" ><span class="btn btn-default" data-toggle="modal" data-target="#qfqModal101" data-title="Download: dl.php/123" data-text="Please wait" data-backdrop="static" data-keyboard="false" onclick="$(\'#qfqModalTitle101\').text($(this).data(\'title\')); $(\'#qfqModalText101\').text($(this).data(\'text\'));"><span class="glyphicon glyphicon-file" ></span></span></a>', $result);
+
+ }
+
/**
* @throws \CodeException
* @throws \UserFormException
diff --git a/extension/ext_conf_template.txt b/extension/ext_conf_template.txt
index 85c6bcade..888a4d3ce 100644
--- a/extension/ext_conf_template.txt
+++ b/extension/ext_conf_template.txt
@@ -390,11 +390,24 @@ custom30 =
# cat=file/file; type=string; label=Query for direct download mode. Access via download.php. No default.: SELECT CONCAT('d:output.pdf|F:', n.pathFileName) FROM notiz AS n WHERE n.id=? AND NOW()<n.expire
sqlDirectdownloadphp =
+# cat=file/file; type=string; label=Message shown to the user if record isn't found.
+sqlDirectdownloadphperror =
+
# cat=file/file; type=string; label=Query for direct download mode. Access via dl.php. No default.: SELECT CONCAT('d:output.pdf|F:', n.pathFileName) FROM notiz AS n WHERE n.id=? AND NOW()<n.expire
sqlDirectdlphp =
+# cat=file/file; type=string; label=Message shown to the user if record isn't found.
+sqlDirectdlphperror =
+
# cat=file/file; type=string; label=Query for direct download mode. Access via dl2.php. No default.: SELECT CONCAT('d:output.pdf|F:', n.pathFileName) FROM notiz AS n WHERE n.id=? AND NOW()<n.expire
sqlDirectdl2php =
+# cat=file/file; type=string; label=Message shown to the user if record isn't found.
+sqlDirectdl2phperror =
+
# cat=file/file; type=string; label=Query for direct download mode. Access via dl3.php. No default.: SELECT CONCAT('d:output.pdf|F:', n.pathFileName) FROM notiz AS n WHERE n.id=? AND NOW()<n.expire
sqlDirectdl3php =
+
+# cat=file/file; type=string; label=Message shown to the user if record isn't found.
+sqlDirectdl3phperror =
+
--
GitLab