Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRM-18756: TrackableURLOpen: fix SQL parameter escaping convention. #8516

Merged
merged 2 commits into from Aug 7, 2016

Conversation

mlutfy
Copy link
Member

@mlutfy mlutfy commented Jun 6, 2016

@mlutfy
Copy link
Member Author

mlutfy commented Jun 8, 2016

I reproduced the testing steps from CRM-17959 to make sure that adding the SQL validation code didn't break anything, and added a test for CRM-17959. Seems OK.

@seamuslee001
Copy link
Contributor

Looking over this, this looks fine to me and i think is ok to be merged. @eileenmcnaughton do you think this is ok? as far as I can tell it isn't actually changing the effect of any of the code just making it more secure

@eileenmcnaughton
Copy link
Contributor

I just tested this by putting it on a site & then clicking on a trackable url from an email - the url worked & took me through to the destination - I think this proves it doesn't break anything - so merging.

I agree with Seamus's reading of the code

wp-content/plugins/civicrm/civicrm/extern/url.php?u=72&qid=8758

@eileenmcnaughton eileenmcnaughton merged commit 7a5a698 into civicrm:master Aug 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants