-
-
Notifications
You must be signed in to change notification settings - Fork 814
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-21214 Fix crashes when 'null' string is passed in via API #11324
CRM-21214 Fix crashes when 'null' string is passed in via API #11324
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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'])) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cheers, updated.
c42166b
to
c320f13
Compare
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait now you've changed the meaning of this conditional. Are you sure about that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No... I've fixed that now.
c320f13
to
6b60d8a
Compare
…_crashes CRM-21214 Fix crashes when 'null' string is passed in via API
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. |
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?