From 9a197f113a3301de33e3fcfbc27f590791d873d1 Mon Sep 17 00:00:00 2001
From: twistor <twistor@473738.no-reply.drupal.org>
Date: Tue, 16 Jun 2015 20:55:28 -0700
Subject: [PATCH] Issue #1848498 by twistor: Respect allowed file extensions in
file mapper
---
mappers/file.inc | 3 +-
plugins/FeedsParser.inc | 119 +++++++++++++++++++++++++++++------
tests/feeds_mapper_file.test | 71 ++++++++++++++++++---
3 files changed, 165 insertions(+), 28 deletions(-)
diff --git a/mappers/file.inc b/mappers/file.inc
index 0c406776..d46c3191 100644
--- a/mappers/file.inc
+++ b/mappers/file.inc
@@ -111,13 +111,14 @@ function file_feeds_set_target(FeedsSource $source, $entity, $target, array $val
case 'uri':
if ($v) {
try {
+ $v->setAllowedExtensions($instance_info['settings']['file_extensions']);
$file = $v->getFile($destination);
$field[LANGUAGE_NONE][$delta] += (array) $file;
// @todo: Figure out how to properly populate this field.
$field[LANGUAGE_NONE][$delta]['display'] = 1;
}
catch (Exception $e) {
- watchdog_exception('Feeds', $e, nl2br(check_plain($e)));
+ watchdog('feeds', check_plain($e->getMessage()));
}
}
break;
diff --git a/plugins/FeedsParser.inc b/plugins/FeedsParser.inc
index c864f291..8b430c05 100644
--- a/plugins/FeedsParser.inc
+++ b/plugins/FeedsParser.inc
@@ -264,7 +264,27 @@ class FeedsGeoTermElement extends FeedsTermElement {
* Enclosure element, can be part of the result array.
*/
class FeedsEnclosure extends FeedsElement {
- protected $mime_type;
+
+ /**
+ * The mime type of the enclosure.
+ *
+ * @param string
+ */
+ protected $mime_type;
+
+ /**
+ * The default list of allowed extensions.
+ *
+ * @param string
+ */
+ protected $allowedExtensions = 'jpg jpeg gif png txt doc xls pdf ppt pps odt ods odp';
+
+ /**
+ * The sanitized local file name.
+ *
+ * @var string
+ */
+ protected $safeFilename;
/**
* Constructor, requires MIME type.
@@ -287,6 +307,17 @@ class FeedsEnclosure extends FeedsElement {
return $this->mime_type;
}
+ /**
+ * Sets the list of allowed extensions.
+ *
+ * @param string $extensions
+ * The list of allowed extensions separated by a space.
+ */
+ public function setAllowedExtensions($extensions) {
+ // Normalize whitespace so that empty extensions are not allowed.
+ $this->allowedExtensions = trim(preg_replace('/\s+/', ' ', $extensions));
+ }
+
/**
* Use this method instead of FeedsElement::getValue() when fetching the file
* from the URL.
@@ -301,20 +332,75 @@ class FeedsEnclosure extends FeedsElement {
}
/**
- * Use this method instead of FeedsElement::getValue() to get the file name
- * transformed for better local saving (underscores instead of spaces)
+ * Returns the full path to the file URI with a safe file name.
*
- * @return
+ * @return string
+ * The safe file URI.
+ *
+ * @throws RuntimeException
+ * Thrown if the file extension is invalid.
+ */
+ public function getSanitizedUri() {
+ return drupal_dirname($this->getValue()) . '/' . $this->getSafeFilename();
+ }
+
+ /**
+ * Returns the file name transformed for better local saving.
+ *
+ * @return string
* Value with space characters changed to underscores.
*
- * @see FeedsElement::getValue()
+ * @throws RuntimeException
+ * Thrown if the file extension is invalid.
*/
public function getLocalValue() {
- return str_replace(' ', '_', $this->getValue());
+ return str_replace(' ', '_', $this->getSafeFilename());
}
/**
- * @return
+ * Returns the safe file name.
+ *
+ * @return string
+ * A filename that is safe to save to the filesystem.
+ *
+ * @throws RuntimeException
+ * Thrown if the file extension is invalid.
+ */
+ protected function getSafeFilename() {
+ if (isset($this->safeFilename)) {
+ return $this->safeFilename;
+ }
+
+ $filename = rawurldecode(drupal_basename($this->getValue()));
+
+ if (module_exists('transliteration')) {
+ require_once drupal_get_path('module', 'transliteration') . '/transliteration.inc';
+ $filename = transliteration_clean_filename($filename);
+ }
+
+ // Remove leading and trailing whitespace and periods.
+ $filename = trim($filename, " \t\n\r\0\x0B.");
+
+ if (strpos($filename, '.') === FALSE) {
+ $extension = FALSE;
+ }
+ else {
+ $extension = substr($filename, strrpos($filename, '.') + 1);
+ }
+
+ if (!$extension || !in_array($extension, explode(' ', $this->allowedExtensions), TRUE)) {
+ throw new RuntimeException(t('The file @file has an invalid extension.', array('@file' => $filename)));
+ }
+
+ $this->safeFilename = file_munge_filename($filename, $this->allowedExtensions, FALSE);
+
+ return $this->safeFilename;
+ }
+
+ /**
+ * Downloads the content from the file URL.
+ *
+ * @return string
* The content of the referenced resource.
*/
public function getContent() {
@@ -345,13 +431,14 @@ class FeedsEnclosure extends FeedsElement {
// Prepare destination directory.
file_prepare_directory($destination, FILE_MODIFY_PERMISSIONS | FILE_CREATE_DIRECTORY);
// Copy or save file depending on whether it is remote or local.
- if (drupal_realpath($this->getValue())) {
+ if (drupal_realpath($this->getSanitizedUri())) {
$file = new stdClass();
$file->uid = 0;
- $file->uri = $this->getValue();
+ $file->uri = $this->getSanitizedUri();
$file->filemime = $this->mime_type;
- $file->filename = basename($file->uri);
- if (dirname($file->uri) != $destination) {
+ $file->filename = $this->getSafeFilename();
+
+ if (drupal_dirname($file->uri) !== $destination) {
$file = file_copy($file, $destination);
}
else {
@@ -368,16 +455,11 @@ class FeedsEnclosure extends FeedsElement {
}
}
else {
- $filename = basename($this->getLocalValue());
- if (module_exists('transliteration')) {
- require_once drupal_get_path('module', 'transliteration') . '/transliteration.inc';
- $filename = transliteration_clean_filename($filename);
- }
if (file_uri_target($destination)) {
$destination = trim($destination, '/') . '/';
}
try {
- $file = file_save_data($this->getContent(), $destination . $filename);
+ $file = file_save_data($this->getContent(), $destination . $this->getLocalValue());
}
catch (Exception $e) {
watchdog_exception('Feeds', $e, nl2br(check_plain($e)));
@@ -388,8 +470,9 @@ class FeedsEnclosure extends FeedsElement {
if (!$file) {
throw new Exception(t('Invalid enclosure %enclosure', array('%enclosure' => $this->getValue())));
}
+
+ return $file;
}
- return $file;
}
}
diff --git a/tests/feeds_mapper_file.test b/tests/feeds_mapper_file.test
index dcf5d981..8a7cef58 100644
--- a/tests/feeds_mapper_file.test
+++ b/tests/feeds_mapper_file.test
@@ -17,6 +17,10 @@ class FeedsMapperFileTestCase extends FeedsMapperTestCase {
);
}
+ public function setUp() {
+ parent::setUp(array('dblog'));
+ }
+
/**
* Basic test loading a single entry CSV file.
*/
@@ -36,7 +40,14 @@ class FeedsMapperFileTestCase extends FeedsMapperTestCase {
// Reset all the caches!
$this->resetAll();
}
- $typename = $this->createContentType(array(), array('files' => 'file'));
+ $typename = $this->createContentType(array(), array(
+ 'files' => array(
+ 'type' => 'file',
+ 'instance_settings' => array(
+ 'instance[settings][file_extensions]' => 'png, gif, jpg, jpeg',
+ ),
+ ),
+ ));
// 1) Test mapping remote resources to file field.
@@ -68,8 +79,7 @@ class FeedsMapperFileTestCase extends FeedsMapperTestCase {
->execute();
foreach ($entities as $entity) {
$this->drupalGet('node/' . $entity->entity_id . '/edit');
- $f = new FeedsEnclosure(array_shift($files), NULL);
- $this->assertText($f->getLocalValue());
+ $this->assertText(str_replace(' ', '_', array_shift($files)));
}
// 2) Test mapping local resources to file field.
@@ -115,8 +125,7 @@ class FeedsMapperFileTestCase extends FeedsMapperTestCase {
foreach ($entities as $entity) {
$this->drupalGet('node/' . $entity->entity_id . '/edit');
- $f = new FeedsEnclosure(array_shift($files), NULL);
- $this->assertRaw('resources/' . $f->getUrlEncodedValue());
+ $this->assertRaw('resources/' . rawurlencode(array_shift($files)));
}
// 3) Test mapping of local resources, this time leave files in place.
@@ -142,8 +151,7 @@ class FeedsMapperFileTestCase extends FeedsMapperTestCase {
foreach ($entities as $entity) {
$this->drupalGet('node/' . $entity->entity_id . '/edit');
- $f = new FeedsEnclosure(array_shift($files), NULL);
- $this->assertRaw('images/' . $f->getUrlEncodedValue());
+ $this->assertRaw('images/' . rawurlencode(array_shift($files)));
}
// Deleting all imported items will delete the files from the images/ dir.
@@ -208,13 +216,58 @@ class FeedsMapperFileTestCase extends FeedsMapperTestCase {
foreach ($entities as $i => $entity) {
$this->drupalGet('node/' . $entity->entity_id . '/edit');
- $f = new FeedsEnclosure(array_shift($files), NULL);
- $this->assertRaw($f->getUrlEncodedValue());
+ $this->assertRaw(str_replace(' ', '_', array_shift($files)));
$this->assertRaw("Alt text $i");
$this->assertRaw("Title text $i");
}
}
+ public function testInvalidFileExtension() {
+ variable_set('feeds_never_use_curl', TRUE);
+
+ $typename = $this->createContentType(array(), array(
+ 'files' => array(
+ 'type' => 'file',
+ 'instance_settings' => array(
+ 'instance[settings][file_extensions]' => 'txt',
+ ),
+ ),
+ ));
+
+ // Create a CSV importer configuration.
+ $this->createImporterConfiguration('Node import from CSV', 'invalid_extension');
+ $this->setPlugin('invalid_extension', 'FeedsCSVParser');
+ $this->setSettings('invalid_extension', 'FeedsNodeProcessor', array('bundle' => $typename));
+ $this->setSettings('invalid_extension', NULL, array('content_type' => ''));
+ $this->addMappings('invalid_extension', array(
+ 0 => array(
+ 'source' => 'title',
+ 'target' => 'title',
+ ),
+ 1 => array(
+ 'source' => 'file',
+ 'target' => 'field_files:uri',
+ ),
+ ));
+
+ // Import.
+ $edit = array(
+ 'feeds[FeedsHTTPFetcher][source]' => url('testing/feeds/files-remote.csv', array('absolute' => TRUE)),
+ );
+ $this->drupalPost('import/invalid_extension', $edit, 'Import');
+ $this->assertText('Created 5 nodes');
+
+ foreach (range(1, 5) as $nid) {
+ $node = node_load($nid);
+ $this->assertTrue(empty($node->field_files));
+ }
+
+ foreach ($this->listTestFiles() as $filename) {
+ $message = t('The file @file has an invalid extension.', array('@file' => $filename));
+ $this->assertTrue(db_query("SELECT 1 FROM {watchdog} WHERE message = :message", array(':message' => $message))->fetchField());
+ }
+ }
+
/**
* Lists test files.
*/
--
GitLab