← Back to team overview

openerp-community-reviewer team mailing list archive

Re: [Merge] lp:~serpentcs/web-addons/multi_image_7.0 into lp:web-addons

 

> Hello Friend.
> 
> I found the support for the security issue, "at least from my point of view".
> 
> Point 1:
> 
> 167     +            file_name = current_dat_time + "_" + ufile.filename
> 
> With this line we brake a Python Standard, we have some libraries to manage
> the file names in a secure way, "Why use dates?".
> 
> One option:
> 
> http://stackoverflow.com/questions/10501247/best-way-to-generate-random-file-
> names-in-python

We set unique file name. if we use uuid python library then may be it will generate duplicate number so it will be conflicts with existing file name. let me check it there another lib for generate unique number not generate duplicate number in any case.

> 
> AFAIK It is already implemented on document_ftp
> 
> Point 2:
> 
> Your module "without ask the user or/and the technicians" is writing over the
> web/module, it is a big security issue even with community addons, you can
> find the support about why name space correctly where you put your files.
> 
> 168     +            addons_path =
> openerpweb.addons_manifest['web']['addons_path'] +
> "/web/static/src/img/image_multi/"

We must be store the image in openerp server directory we cannot set file any other directory because for displaying image we just give path for display image in web-browser. 
> 
> Option 1:
> In this case you can use the static_http_document_root in your server config
> file, and if it is not setted ask for one with a raise may be.
> Option 2:
> Extend the document module giving the access to the folders to physical
> folders.
if we store file in any other directory on server like path store in config file or use static_http_document_root then we can store file on that path so it's time consuming it's take much time for loading images. User open the all images then fetch the image datas using rpc call and display image base on datas.
> 
> IOH, FYI, we are working on it, and we think it is a best approach, do you
> think we can merge the concepts in your module?
> 
> 
> http://www.w3.org/Protocols/rfc2616/rfc2616-sec15.html [[Section 15.2]]
> 
> IMHO, it should be done extending the dms in OpenERP (with your approach of
> upload and make public), if you have some point against it it should be great
> if you share your thoughts.

DMS also store file in server directory with random unique file name.

We will improve module for security reason.

> For now and until we can not solve the security issues (or support them)
> 
> ICONS: Please use generic icons with a little mention to your company
> specifying the use of the module, avoind use your branding __only__
> 
> Descriptors.
> 
> Can you please explain a little how use the module in the descriptor?
> 
> Conclusion,
> 
> For me it is a great approach __really__ but it should be great if you can
> argue my points or improve in your code, please (or better put the branch with
> openerp-community owner and we can help ;-)  )

We will Improve code as per your suggestions and we will add description in openerp.py file too.

Thanks for nice suggestions.
-- 
https://code.launchpad.net/~serpentcs/web-addons/multi_image_7.0/+merge/179857
Your team Web-Addons Core Editors is subscribed to branch lp:web-addons.


Follow ups

References