Now filefields support only remote files.
I need support for local files - in my case xml source file and images are already on the server.

So I modified FeedsParser.inc: if url is a relative path consider it relative to feeds upload directory (e.g. sites\default\files\feeds). take a file from there and use it directly.

CommentFileSizeAuthor
#13 755556-13_local_files.patch1.22 KBalex_b
#11 755556-11_local_files.patch1.12 KBalex_b
#10 755556-10_relative_files.patch2.51 KBandrewlevine
#6 755556-6_relative_files.patch2.96 KBalex_b
#1 FeedsParser.inc-755556.patch1.71 KBMonkey Master

Comments

Monkey Master’s picture

Monkey Master commented
StatusFileSize
new FeedsParser.inc-755556.patch1.71 KB
alex_b’s picture

alex_b commented
Status: Active » Closed (won't fix)

Here is the situation: For security reasons, only files from the site's upload directory are allowed. The reason is that otherwise a user who is allowed to use a file fetcher based importer would be incredibly close to reading any files that on a server that the web server has access to. So I'd like to avoid that.

See FeedsFileFetcher::sourceFormValidate() how it checks for the file's location if the file is specified directly.

I am afraid this is a won't fix. For your specific use case you could consider extending FeedsFileFetcher class. I can't recommend it and you'd need to make sure that users that can access this importer are trusted.

I will document this better in the code.

alex_b’s picture

alex_b commented

Better comment clarifying security situation: http://drupal.org/cvs?commit=347560

Monkey Master’s picture

Monkey Master commented
Status: Closed (won't fix) » Active

It's not about FeedsFileFetcher - it's about filefield mapper which uses FeedsEnclosure limited to remote files only - unlike FeedsFileFetcher.

I modified FeedsEnclosure->getFile() to also work with local files in site's upload directory - I don't see a security problem here...

class FeedsEnclosure extends FeedsElement {
...
  public function getFile() {
    if(!isset($this->file)) {
      if (!preg_match('@^((ftp|https?)://|/)@', $this->getValue())) {
        $this->file = file_directory_path() . '/feeds/' . $this->getValue();
        if (!is_file($this->file)) {
          throw new Exception(t('File %name not found.', array('%name' => $this->getValue())));
        }
...

I can also add file_check_location($this->file, file_directory_path()) - like in FeedsFileFetcher::sourceFormValidate() - to prevent using ../ in $this->getValue()

alex_b’s picture

alex_b commented
Status: Active » Needs review

Um sorry. My only excuse is that it was already late when I reviewed : ) I'll get back to this.

alex_b’s picture

alex_b commented
Version: 6.x-1.0-alpha12 » 6.x-1.x-dev
Status: Needs review » Needs work
StatusFileSize
new 755556-6_relative_files.patch2.96 KB

- Improved comments.
- Added checks for location of file targets.
- Working against latest HEAD.

Some assertions of "Mapper: FileField" test are failing.

NW. I won't be able to come back and work on this patch in a while. Will be able to review though...

rjbrown99’s picture

rjbrown99 commented

Two more issues related to getFile and paths and filenames. Perhaps we should consolidate all of the getFile stuff to one issue? All of these now have patches as well.

#706908: Enhance filefield mapper to perform more validation on remote URLs
#840350: (Optionally) Transliterate enclosure filenames to provide protection from awkward names

alex_b’s picture

alex_b commented
Status: Needs work » Needs review

#7 - they're all fairly different: better validation of URLs, transliterate file names and support of local files. Therefore, I wouldn't merge them.

alex_b’s picture

alex_b commented
Status: Needs review » Needs work

Didn't mean to change status.

andrewlevine’s picture

andrewlevine commented
Status: Needs work » Needs review
StatusFileSize
new 755556-10_relative_files.patch2.51 KB

Building on Alex's patch from #6

alex_b’s picture

alex_b commented
StatusFileSize
new 755556-11_local_files.patch1.12 KB

Couldn't this be muuuch simpler?

andrewlevine’s picture

andrewlevine commented
Status: Needs review » Needs work

@alex_b #11

Yes, that looks great. One thing I think is necessary is to add a file_exists() call because file_check_location returns a path even if the file doesn't exist.

alex_b’s picture

alex_b commented
Status: Needs work » Needs review
StatusFileSize
new 755556-13_local_files.patch1.22 KB

There you go.

andrewlevine’s picture

andrewlevine commented

Looks good to me. Feel free to commit now if you're comfortable. Either way I will be testing the patch next week.

andrewlevine’s picture

andrewlevine commented
Status: Needs review » Reviewed & tested by the community

I have tested this in my own application (extending FeedsEnclosure to make sure the right file path is passed in) and saving local files works as advertised.

If this passes the tests I believe it's ready

alex_b’s picture

alex_b commented
Status: Reviewed & tested by the community » Fixed

This is committed, thank you.

http://drupal.org/cvs?commit=422356

alex_b’s picture

alex_b commented

This feature is actually broken in beta7, as referenced files get automatically deleted: #919172: Fix enclosure source files deleted

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

micheleannj’s picture

micheleannj commented

Can anyone confirm if this is working in beta10?