Problem/Motivation

It’s possible (I know - I’ve just spent days trying to understand the problem!) that the json data is not in utf8 format and that will mean that json_decode() will just return NULL. See http://php.net/manual/en/function.json-decode.php#86997

Proposed resolution

Just before we call json_decode(), we should call utf8_encode() on the response we have.

Remaining tasks

Patch, review

User interface changes

none.

API changes

none.

Data model changes

none, other than we stand a better chance of working with a wider variety of data sources.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rachel_norfolk created an issue. See original summary.

rachel_norfolk’s picture

And a patch to review...

rachel_norfolk’s picture

Status: Active » Needs review

Helpful, when adding a patch, to remember to change the status...

rachel_norfolk’s picture

Oh - just noticed that 8.x-2.x—dev is quite a bit different from beta 2 now. Here’s the same patch agains beta 2, if that is useful. (set to not be tested, hopefully, just for reference)

mikeryan’s picture

Status: Needs review » Fixed

Committed, thanks!

  • mikeryan committed 389c4c3 on 8.x-3.x
    Revert Issue #2799353 by rachel_norfolk: json_decode() assumes utf8...

  • mikeryan committed 7f2b2b3 on 8.x-2.x
    Revert Issue #2799353 by rachel_norfolk: json_decode() assumes utf8...
mikeryan’s picture

Status: Fixed » Active

Reverted per #2817099: Special characters are imported incorrectly during migration - seems this may do more harm than good.

mikeryan’s picture

Status: Active » Needs work
rachel_norfolk’s picture

Status: Needs work » Needs review
FileSize
1003 bytes
1.14 KB

Yes - I can see now why this might not be ideal when a valid result might be obtained without recourse to utf8_endode().

What we should do I *only* do the encoding if json_decode() is failing and returning NULL.

More intelligent (!) patch attached.

rachel_norfolk’s picture

Also - mikeryan - note that tests can only be run against 8.x-2.x currently, not yet 3.x

mikeryan’s picture

Status: Needs review » Needs work

Ran the 8.x-3.x test (not that this is actually tested:P).

I do need to point out:

  1. According to https://tools.ietf.org/html/rfc7159, "JSON text SHALL be encoded in UTF-8, UTF-16, or UTF-32."
  2. utf8_encode() only converts ISO-8859-1 to UTF-8 (so, if it works for you, you must have ISO-8859-1 text).

I.e., what you have is not legal JSON.

That being said, from the comments at http://php.net/manual/en/function.json-decode.php it seems that brain-dead webservices producing ISO-8859-1 aren't all that unusual so we might as well guard against them...

+++ b/src/Plugin/migrate_plus/data_parser/Json.php
@@ -42,8 +42,17 @@ class Json extends DataParserPluginBase implements ContainerFactoryPluginInterfa
+    if ($source_data == NULL) {

Should use is_null() instead of comparison to NULL.

mikeryan’s picture

Status: Needs work » Fixed

Went ahead and fixed the is_null() on commit, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.