I have some problems with the parsed data, when I would like to save an image in Drupal from an external source. I use the latest dev release of Feeds 7.x-2.x and Feeds XML Parser.

I relaized that all of the parsed data is structured like that: "\nSOME VALUE\n"

It seems that the file_save_data() can't handle the starting and ending new line and throw an error. If I trim the return value of getValue() method in FeedsElement class, the problem disappears.

Change code
from:

public function getValue() {
  return $this->value;
}

to:

public function getValue() {
  return trim($this->value);
}

Comments

twistor’s picture

Well, trim isn't quite right since it can remove whitespace as well. Just removes tabs and newlines? Maybe we want to remove spaces, but they are valid in a filesystem. hmmm

helmo’s picture

Status: Active » Needs review
StatusFileSize
new563 bytes

Here's a patch for that change ...

It has the same trim in FeedsTermElement:getValue ... not sure if that's needed...

helmo’s picture

megachriz’s picture

Status: Needs review » Needs work

As twistor says in #1, trim() isn't quite right as spaces are valid for being used in filenames. We should have a patch that only removes tabs and newlines.

Also, you could use Feeds Tamper to fix values from the source.

megachriz’s picture

Issue tags: +Needs tests

Seems it is quite easy: we could just specify the characters to trim as second parameter for that function:
http://php.net/trim

This could also use an automated test to demonstrate the problem. This way we ensure this error will not return in the future.

megachriz’s picture

Issue tags: +Novice
mike_pol’s picture

Assigned: Unassigned » mike_pol
Status: Needs work » Needs review
StatusFileSize
new579 bytes

Added filter to trim() so it only trims tabs and new lines

megachriz’s picture

Thanks for the patch, mike_pol! Patch looks good to me.

Here is a patch with an adjusted test file that is used to for testing importing image files. I added a tab and two new lines to it. With the fix applied, all tests should pass. The tests only patch should fail tests.

megachriz’s picture

StatusFileSize
new1.36 KB
new817 bytes

Hm, the tab did make it in the tests-only patch, but not in the other patch with the fix included. Regenerated that patch and the interdiff.

The last submitted patch, 8: feeds-new-line-parsed-url-2202817-8-tests-only.patch, failed testing.

The last submitted patch, 8: feeds-new-line-parsed-url-2202817-8.patch, failed testing.

bluegeek9’s picture

Assigned: mike_pol » Unassigned
Status: Needs review » Closed (outdated)
Issue tags: -Needs tests, -Novice

Drupal 7 reached end of life and the D7 version of Feeds is no longer being developed. To keep the issue queue focused on supported versions, we’re closing older D7 issues.

If you still have questions about using Feeds on Drupal 7, feel free to ask. While we won’t fix D7 bugs anymore, we’re happy to offer guidance to help you move forward. You can do so by opening (or reopening) a D7 issue, or by reaching out in the #feeds channel on Drupal Slack.

If this issue is still relevant for Drupal 10+, please open a follow-up issue or merge request with proposed changes. Contributions are always welcome!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.