The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Now feedapi only allows schemes that are valid according to valid_url.
Should be fixed in 1.9
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | 552222-17_secure_file.patch | 2.39 KB | alex_b |
| #15 | 552222-1_secure_file_proto.patch | 2.44 KB | aron novak |
| #12 | 552222_secure_file_proto.patch | 2.44 KB | aron novak |
| #9 | 552222_readme_warning.patch | 837 bytes | aron novak |
| #8 | feedapi.patch | 1.88 KB | killes@www.drop.org |
Comments
Comment #1
aron novakPatch is here thanks to killes@www.drop.org:
http://drupal.org/files/issues/feedapi_5.patch
Comment #2
killes@www.drop.org commentedThat patch should maybe be a changed to extend the description for the form field. Not sure what to write, though.
Comment #3
killes@www.drop.org commentedHere's a patch which also allows this for the drush integration.
Comment #4
aron novak#3: great, drush integration is also an important part of the module ;)
I extended it with an explanation in the description of URL field.
It's RTBC from my point of view. Do you agree?
Comment #5
killes@www.drop.org commentedThe English help text seemed a bit strange, I've updated it to something that I believe to be better.
Comment #6
aron novakThank you for cooperating on this! (and also for the language improvements!)
Comment #8
killes@www.drop.org commentedSince the usage of local files is possibly a security issue, we should have a special permission for them, see patch. We should add a remark in the README as well, but I am not sure where you want to place that.
Comment #9
aron novakIf you also like the text of the README, it's RTBC.
Comment #10
alex_b commentedI know we have talked about this before, but revisiting this, I start thinking that we should limit possible files to the site's files directory. Killes: would this address your use case?
Addressing this issue remains critical. We can't roll a release without fixing this.
If we go down the proposed extra permission route, we should:
#8: rename 'allow local files as feeds' to 'use local files as feeds' - makes more sense from a role point of view.
#9: adjust README text accoringly.
Comment #11
aron novakThere is a problem with the form approach, node/add/feed and drush is not the only point where the user can enter feed URL. There is a block as well.
Comment #12
aron novakAttached a new patch, simplified form validating is included, restricted to the files/ directory.
drush create command is not a problem, if someone has shell access, it's not a problem if he can use local file urls in a non-restricted way
Comment #13
aron novakREADME update conflicts w/ #500856: Clean up and improve README, so i added the notes there.
Comment #14
alex_b commentedJust talked to Jose Reyero.
He pointed me at:
http://api.drupal.org/api/function/file_check_location/6
We should use that instead of doing our own check.
He also suggested to make the directory configurable, so that you can point it to any other directory if necessary. I think we could add this functionality after this patch has been committed, especially if killes is fine with the limitation of the drupal files directory.
Comment #15
aron novakNow it uses file_check_location
Comment #16
alex_b commentedShould be:
To make sure that file:// is really at the beginning of the string.
Similarly, let's use substr() instead of str_replace() to replace the first 7 characters that compose file://
Comment #17
alex_b commentedIncorporated suggestions from #16
Comment #18
aron novakComment #19
alex_b commentedIn the face of the fact that we need to roll a FeedAPI release ASAP (#580508: FeedAPI doesnt update on cron run after drupal 6.14 upgrade) I have committed this now.
I know that this patch may be a little too tight for what killes is trying to do. If so, let's deal with the limitations in a separate follow up issue. Setting to fixed.
Thank you everybody.