Code review comment for lp:~stefanor/ibid/rfc-plugin-330864

Revision history for this message
Jonathan Hitchcock (vhata) wrote :

10:30 -+- Vhata reads cacheable_download()
10:30 <Vhata> iiiiinteresting
10:31 <Vhata> what do you think about cacheable_download(url, cachefilename), and then cachefile = "%s/%s" % (config['cachedir'], cachefilename), after validating cachefilename to exclude '/' ?
10:35 <tumbleweed> Vhata: firstly, os.path.join
10:35 <tumbleweed> Vhata: suits me, although I'm tempted to do cachedir/plugin/filename
10:36 <Vhata> sorry, you're right :)
10:36 <Vhata> um
10:36 <Vhata> multiple plugins might want the same file
10:36 <Vhata> buuuut
10:36 <Vhata> multiple plugins might want different files with the same name
10:36 <Vhata> buuuuut
10:37 <Vhata> how do you know which plugin is calling it?
10:38 <tumbleweed> Vhata: good point
10:38 <tumbleweed> how about having that as a standard guideline
10:38 <tumbleweed> so if you write a plugin, you should request pluginname/file
10:39 <tumbleweed> that'll also allow you to poke in other people's files if you know what you are doing
10:43 <Vhata> plugins should kinda be independent
10:43 <Vhata> which invalidates my first point
10:43 <Vhata> so, ja, request pluginname/file
10:44 <Vhata> and validating cachefilename isn't really necessary, because plugins aren't supposed to be security risks, surely
10:46 <tumbleweed> my thoughts too
10:46 <tumbleweed> although detecting ".." and "~" would probably do the trick

review: Needs Fixing

« Back to merge proposal