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);
}
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | interdiff-2202817-7-9.txt | 817 bytes | megachriz |
| #9 | feeds-new-line-parsed-url-2202817-9.patch | 1.36 KB | megachriz |
| #8 | feeds-new-line-parsed-url-2202817-8-tests-only.patch | 817 bytes | megachriz |
| #7 | new_line_in_parsed_url-2202817-7.patch | 579 bytes | mike_pol |
| #2 | new_line_in_parsed_url-2202817-2.patch | 563 bytes | helmo |
Comments
Comment #1
twistor commentedWell, 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
Comment #2
helmo commentedHere's a patch for that change ...
It has the same trim in FeedsTermElement:getValue ... not sure if that's needed...
Comment #3
helmo commentedComment #4
megachrizAs 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.
Comment #5
megachrizSeems 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.
Comment #6
megachrizComment #7
mike_pol commentedAdded filter to trim() so it only trims tabs and new lines
Comment #8
megachrizThanks 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.
Comment #9
megachrizHm, 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.
Comment #12
bluegeek9 commentedDrupal 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!