diff --git a/mappers/file.inc b/mappers/file.inc index 646d055..c0350e3 100644 --- a/mappers/file.inc +++ b/mappers/file.inc @@ -76,11 +76,13 @@ function file_feeds_set_target($source, $entity, $target, $value) { $i = 0; $field = isset($entity->$target) ? $entity->$target : array(); foreach ($value as $v) { + $file = FALSE; try { + $v->setAllowedExtensions($instance_info['settings']['file_extensions']); $file = $v->getFile($destination); } catch (Exception $e) { - watchdog_exception('Feeds', $e, nl2br(check_plain($e))); + watchdog('feeds', check_plain($e->getMessage())); } if ($file) { $field['und'][$i] = (array)$file; diff --git a/plugins/FeedsParser.inc b/plugins/FeedsParser.inc index 13fcaa0..3183383 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. @@ -288,6 +308,17 @@ class FeedsEnclosure extends FeedsElement { } /** + * 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,74 @@ 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 filen ame. * - * @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. * + * @throws RuntimeException + * Thrown if the file extension is invalid. + * * @see FeedsElement::getValue() */ 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. + * + * @see FeedsElement::getValue() + */ + 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."); + + $extension = substr($filename, strrpos($filename, '.') + 1); + + if (!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 +430,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 +454,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,9 +469,11 @@ 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 20c7912..7777971 100644 --- a/tests/feeds_mapper_file.test +++ b/tests/feeds_mapper_file.test @@ -21,6 +21,10 @@ class FeedsMapperFileTestCase extends FeedsMapperTestCase { ); } + public function setUp() { + parent::setUp(array('dblog')); + } + /** * Basic test loading a single entry CSV file. */ @@ -40,7 +44,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. @@ -72,8 +83,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. @@ -118,8 +128,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->assertRaw('resources/' . $f->getUrlEncodedValue()); + $this->assertRaw('resources/' . rawurlencode(array_shift($files))); } // 3) Test mapping of local resources, this time leave files in place. @@ -144,8 +153,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->assertRaw('images/' . $f->getUrlEncodedValue()); + $this->assertRaw('images/' . rawurlencode(array_shift($files))); } // Deleting all imported items will delete the files from the images/ dir. @@ -156,6 +164,52 @@ class FeedsMapperFileTestCase extends FeedsMapperTestCase { // } } + 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. */