diff --git a/mappers/file.inc b/mappers/file.inc index 0c406776faedf30959d33ce74da2975970b19d80..d46c31916ffe4d51094a35ea28bddd158339c8c9 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 c864f291738cf0967ad0938a77a0877c22173151..8b430c057914bf6947bf96c2fbb7d43e925b4544 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 dcf5d98149402a1d98ab8939f4bf1b33f6e32aad..8a7cef584221a5eac802e4117504cf634e9ff994 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. */