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

Update to Datahelper.php in 4.4.0 introduced new bug when values are arrays (e.g. category fields) #1041

Closed
kristiansp opened this issue Oct 6, 2021 · 3 comments
Labels

Comments

@kristiansp
Copy link
Contributor

Description

The update done for v 4.4.0 to avoid stripping values of leading zeros introduced a new bug when the values being compared are arrays, which happens when comparing e.g. category fields. The comparison will then error out, as mb_strlen() is passed arrays, and only accepts strings.

The reason is the checks with mb_strlen() that's being done in the new _compareSimpleValues() method in DataHelper.php line 317.

It needs to be adjusted so that it only checks the length if the values passed in are actually strings.

Steps to reproduce

  1. Upgrade to v 4.4.0
  2. Run feed that compares array values (e.g. category fields)

Additional info

  • Craft CMS version: 3.7
  • Feed Me version: 4.4.0
  • PHP version:
  • Database driver & version:
  • Other plugins & versions:
@kristiansp kristiansp added the bug label Oct 6, 2021
@green17
Copy link

green17 commented Dec 6, 2021

We have got this happing to us now too. Same problem I think category fields.

@angrybrad
Copy link
Member

angrybrad commented Dec 6, 2021

@kristiansp @green17

Can you vendor/craftcms/feed-me/src/helpers/DataHelper.php, find _compareSimpleValues, replace it with this and verify that fixes it for you?

private static function _compareSimpleValues($fields, $key, $firstValue, $secondValue): bool
{
    /** @noinspection TypeUnsafeComparisonInspection */
    // Should probably do a strict check, but doing this for backwards compatibility.
    if (Hash::check($fields, $key) && ($firstValue == $secondValue)) {
        // If this is a string, check the lengths
        if (is_string($firstValue) && is_string($secondValue)) {
            // String length comparison to take into account "637" and "0637"
            if (mb_strlen($firstValue) == mb_strlen($secondValue)) {
                return true;
            }

            return false;
        }

        // An array, but loosely equal
        return true;
    }

    // Didn't
    return false;
}

@angrybrad
Copy link
Member

Cut a 4.4.1.1 release with this fixed based on feedback from #1067

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants