CRM-21214 Fix crashes when 'null' string is passed in via API #11324
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
colemanw
reviewed
Nov 24, 2017
CRM/Core/BAO/Address.php
Outdated
| @@ -153,7 +153,7 @@ public static function add(&$params, $fixAddress) { | |||
| } | |||
|
|
|||
| // (prevent chaining 1 and 3) CRM-21214 | |||
| if (CRM_Utils_Array::value('master_id', $params)) { | |||
| if (!CRM_Utils_System::isNull($params['master_id'])) { | |||
There was a problem hiding this comment.
Changing CRM_Utils_Array::value to CRM_Utils_System::isNull has another implication in that php will throw a warning if the array item is not set at all. To avoid this do
if (isset($params['master_id']) && !CRM_Utils_System::isNull($params['master_id'])) {
mattwire
force-pushed
the
CRM-21214_followup_fix_crashes
branch
2 times, most recently
from
c42166b
to
c320f13
Compare
colemanw
reviewed
Nov 25, 2017
CRM/Core/BAO/Address.php
Outdated
| @@ -537,7 +537,7 @@ public static function &getValues($entityBlock, $microformat = FALSE, $fieldName | |||
| $values['display'] = $address->display; | |||
| $values['display_text'] = $address->display_text; | |||
|
|
|||
| if (is_numeric($address->master_id)) { | |||
| if (isset($params['master_id']) && !CRM_Utils_System::isNull($address->master_id)) { | |||
There was a problem hiding this comment.
Wait now you've changed the meaning of this conditional. Are you sure about that?
There was a problem hiding this comment.
No... I've fixed that now.
mattwire
force-pushed
the
CRM-21214_followup_fix_crashes
branch
from
c320f13
to
6b60d8a
Compare
sluc23
pushed a commit
to ixiam/civicrm-core
that referenced
this pull request
Jan 10, 2018
…_crashes CRM-21214 Fix crashes when 'null' string is passed in via API
urlisse
added a commit
to coopsymbiotic/webform_civicrm
that referenced
this pull request
Mar 20, 2018
This fixes the issue of address fields that won't update with CiviCRM 4.7.28. Maybe some other changes are still required regarding this module's usage of null values. However, it seems that core is set to also accept `'null'` strings soon along with `NULL` values... https://www.drupal.org/project/webform_civicrm/issues/2948459 civicrm/civicrm-core#11324
|
@mattwire Thanks very much for this, we just ran aground on this, and your fix is greatly appreciated. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This is a followup fix for the changes made in #11019.
Before
When using Address.create API with master_id = 'null' (string) CiviCRM crashes.
After
When using Address.create API with master_id = 'null' (string) CiviCRM does not crash.
Technical Details
The string 'null' is passed in via the CiviCRM API and we need to catch that otherwise SQL functions crash as they are expecting an integer.
@colemanw As merger of the original PR (#11019) could you take a look at this?