New forms contribution

capminds wrote on Friday, November 16, 2012:

Here is the initial contribution from CapMinds Technologies.In this commit we contribute 3 new forms to OpenEMR community.They are

1.Aftercare plan

2.Transfer summary

3.Treatment plan.

Please review the submitted code

https://github.com/chealth/openemr/commit/a3359c0824ce2622364dca30caa397017e983d62

Tracker link here.

https://sourceforge.net/tracker/?func=detail&aid=3587863&group_id=60081&atid=1245239

Thanks,

Naina.
CapMinds Technologies
www.capminds.com

yehster wrote on Friday, November 16, 2012:

First glance shows me that you need to update your code to use the new security model.
Everything you presented has SQL Injection Vulnerabilities.
http://www.open-emr.org/wiki/index.php/Codebase_Security

bradymiller wrote on Saturday, November 17, 2012:

Hi Capminds,

I just have a quick question before I review this code. Was it created with a form maker (if so, was it formscript.pl or xmlformgen)? (knowing this just helps me when reviewing and looking through the forms code)

thanks,
-brady
OpenEMR

capminds wrote on Wednesday, November 21, 2012:

Brady,
           We created those all 3 forms based on the OpenEMR predefined forms,(i.e misc_billing_option).Its a custom form,so we didn’t use  formscript.pl and xmlformgen.

As per yehster comments we applied the new security module rules into Aftercare plan forms,Updated commit link is here.

https://github.com/chealth/openemr/commit/aef5bc4fb616bf1fa647b07f942bd1f36c701219.

Please review the initial one form.(Aftercare plan).And let us know your comments.Will proceed the remaining two forms.

Naina
CapMinds Technologies
www.capminds.com

sunsetsystems wrote on Saturday, November 24, 2012:

I posted a question in the preceding commit.

Rod
www.sunsetsystems.com

capminds wrote on Saturday, December 01, 2012:

Hi all,
          Here is the updated forms commit link for your  reference.please review this code and let us know your comments on this

https://github.com/capmindstech/openemr/commit/09d31cd7e07c109839b311376838b1bcb931463c

Naina
CapMinds Technologies
www.capminds.com

bradymiller wrote on Saturday, December 22, 2012:

Hi Naina,

I placed a review on github.

thanks for being so patient,
-brady
OpenEMR

capminds wrote on Thursday, January 03, 2013:

Brady,
          
           We made changes as per your comments,Please have a look at once and let us know

Here is the commit link on github

https://github.com/capmindstech/openemr/commit/01a64a43cf6e8ef8a3d651a648836074299dec15

Naina
CapMinds Technologies
www.capminds.com

capminds wrote on Friday, January 04, 2013:

Brady,
            Latest revision of the codes for your review,Please let us know your comments.

-Naina
www.capminds.com

capminds wrote on Friday, January 04, 2013:

Brady,
            Here is git commit link

https://github.com/capmindstech/openemr/commit/860a386f689641bb5e95a0ffcaa2145950e2562f

-Naina

bradymiller wrote on Monday, January 07, 2013:

Hi Naina,
Placed a review on github,
thanks.
-brady
OpenEMR

capminds wrote on Wednesday, January 09, 2013:

Brady,
             Here is updated  commit link for your review,Please let us know your comments on this.

https://github.com/capmindstech/openemr/commit/a199f9f3a7dd5bfbc01f3c92fede25d60282711c

Thanks,
Naina
www.capminds.com

bradymiller wrote on Wednesday, January 09, 2013:

Hi,

Looks good overall and thanks for the contribution. Committed this to the official codebase followed by a commit that fixes several minor issues:
http://github.com/openemr/openemr/commit/6aef7ce6a33ebd91111030713cd328c9872b6876

I also just realized that you are nesting formData() functions within the add_escape_custom() functions in the save.php scripts, which is incorrect (it will cause double escaping of magic quotes and of sql escaping). See my three comments in the following commit: http://github.com/openemr/openemr/commit/2f7b2534a332758f8835fcf47b5090b79f69a424
(search for bradymiller to see them)

Please fix these by removing the formData function calls and replacing variables within the add_escape_custom() with $_POST. Also test it a bit to ensure the fields are populating after doing this.

And to rest of community, I have placed these three forms within the interface/forms directory for now since they are good forms to “replicate” since they 100% follow the new security model. But will leave it up to community if it makes more sense to move them to the contrib/forms directory.

thanks,
-brady
OpenEMR

capminds wrote on Wednesday, January 09, 2013:

Brady,
            Bug fix for contributed forms as per your advise.Please have a look at once and let us know

https://github.com/capmindstech/openemr/commit/82eecd5e16a8d2e724e3170a6c4b2bf3d8703d90

Thanks,
Naina
www.capminds.com

bradymiller wrote on Wednesday, January 09, 2013:

Hi Naina,

Thanks for the contribution and the quick bug fix. I committed your bug fix to sourceforge and I also added one more minor bug fix following your commit (since using the add_escape_custom() function, need to include the library/formdata.inc.php library).

thanks,
-brady
OpenEMR