Skip to content

Fix DB $binds property pollution issues#7

Open
kzap wants to merge 11 commits intoEden-PHP:v3from
examinecom:v1
Open

Fix DB $binds property pollution issues#7
kzap wants to merge 11 commits intoEden-PHP:v3from
examinecom:v1

Conversation

@kzap
Copy link
Copy Markdown
Contributor

@kzap kzap commented Jan 12, 2016

fixes for eden v3, you still have to keep a branch for v1 or v3 or whatever was pre v4 so commits and updates can still be submitted for the old eden...

@kzap
Copy link
Copy Markdown
Contributor Author

kzap commented Jan 15, 2016

can create a v3 branch and merge there

kzap added 2 commits May 19, 2017 13:48
…e it will be polluted the next time the database object is reused in the same script

- created a clearBinds() method
- also clearBinds() if the query fails
- dont use $this->binds = array(), use clearBinds() method
- Calling search()->getQuery() multiple times will now return the same result because we clear binds!
@kzap kzap changed the title better error reporting when query fails Fix DB $binds property pollution issues May 19, 2017
@kzap
Copy link
Copy Markdown
Contributor Author

kzap commented May 19, 2017

Pls create a v1 branch and merge this into it

$binds on the database object gets re-used across all database queries since its normal not to instantiate the database object per query.

The only place $binds is cleared is if query() succeeds, but at any point before that it can fail and if you catch the exception and run the query again using the same database object, your $binds will be polluted with the values from the last query.

So what I did was cleared out the $binds property anywhere it was used before we start binding values, that way its always clean, and if an exception occurs, we clear it also.

v4 branch will need this also probably

@cblanquera
Copy link
Copy Markdown

This is eden v1 https://github.com/openovate/eden

@kzap kzap changed the base branch from master to v3 May 19, 2017 08:07
@kzap
Copy link
Copy Markdown
Contributor Author

kzap commented May 19, 2017

has extra commits because v3 should be up to 22ce4e8
then it became v4

@kzap
Copy link
Copy Markdown
Contributor Author

kzap commented May 19, 2017

ok this is for v3 branch then, updated PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants