← 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 Yogesh.

COmments inline (it should be grate in the MP) ;-)


2013/9/28 Yogesh (SerpentCS) <yogesh.sakhreliya@xxxxxxxxxxxxx>

> > 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.
>

Yes i got it, but the "datetime"  imho is an ugly solution, we can use
hash, uuid, ond / or your own algorithm It is my point, in the link you
have the 3 options explained.

>
> >
> > 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.
>

Noup, it is not true, you can use the "static" folder of any module, not
just from web, it means, if we install a new module, i can not "Without
ask" save in the server folder.

document module have a exchangeable folder with an configured by default,
if you think it is possible, We are working this week on it, we can share
efforts to do that.


> >
> > 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.
>

Noup, I think i can not explain by myself well.

The document module give you the ability to "index" all uploaded files, the
file storage is extensible, and you can serve them directly http (exactly
as the static folder does) then, IMHO it is a matter of dedicate some time
to it.

Even we have a ir.attachment type called "url" that can be used to share it
trhought http.

It is a littel more complicated but by far more secure, due to in "Creation
TIme" you can manage the own ACL of OpenERP and in "Serve TIme" just serve
them with an standard http request, (the webclient is doing with this
technique.

I hope it is more clear.


> >
> > 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 Sure, but I can change the default directory in my own deployment it is
not wired in code, it is our point.

>
> > 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.
>

Thanks you for the contribution dude.

> --
>
> https://code.launchpad.net/~serpentcs/web-addons/multi_image_7.0/+merge/179857
> You are reviewing the proposed merge of
> lp:~serpentcs/web-addons/multi_image_7.0 into lp:web-addons.
>



-- 
--------------------
Saludos Cordiales

Nhomar G. Hernandez M.
+58-414-4110269
Skype: nhomar00
Web-Blog: http://geronimo.com.ve
Servicios IT: http://vauxoo.com
Linux-Counter: 467724
Correos:
nhomar@xxxxxxxxxxxxxx
nhomar@xxxxxxxxxx
twitter @nhomar

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.


References