consolidation / site-process

A thin wrapper around the Symfony Process Component that allows applications to use the Site Alias library to specify the target for a remote call.
Other
50 stars 19 forks source link

Windows: ProcessBase.php line 172: Unable to decode output into JSON: Syntax error #56

Open Ambient-Impact opened 2 years ago

Ambient-Impact commented 2 years ago

Describe the bug or behavior Running drush drupal:directory always results in an exception:

E:\Web\Ambient.Impact\Web (4.x -> origin) (ambientimpact-drupal@1.0.0)
λ drush drupal:directory ambientimpact_base

In ProcessBase.php line 172:

  Unable to decode output into JSON: Syntax error

  {
      "%paths": {
          "%root": "E:\Web\Ambient.Impact\Web/drupal",
          "%site": "sites/default",
          "%modules": "sites/all/modules",
          "%themes": "sites/all/themes",
          "%config-sync": "E:\Web\Ambient.Impact\Web/drupal/../drupal_config/sync",
          "%files": "sites/default/files",
          "%temp": "C:\Users\i\AppData\Local\Temp",
          "%private": "E:\Web\Ambient.Impact\Web/drupal/../drupal_private_files",
          "%ambientimpact_base": "E:\Web\Ambient.Impact\Web/drupal/themes/ambientimpact/ambientimpact_base"
      }
  }

To Reproduce See above.

Expected behavior Not a horrible error. :P

Actual behavior See output above.

Workaround After a bit of digging and trial and error, I discovered that commenting out line 162 in ProcessBase.php fixes the issue for some reason, i.e. this line:

$output = preg_replace('#\\\\{2}#', '\\', $output);

Then running the same Drush command completes successfully:

E:\Web\Ambient.Impact\Web (4.x -> origin) (ambientimpact-drupal@1.0.0)
λ drush drupal:directory ambientimpact_base
E:/Web/Ambient.Impact/Web/drupal/themes/ambientimpact/ambientimpact_base

System Configuration

Q A
Drush version? 10.6.1
Drupal version? 9.3.0
PHP version 7.3.15
OS? Windows 10 Pro, version 21H1, build 19043.1415

Additional information Possible related, posted comment: drush-ops/drush#4281

thirstysix commented 2 years ago

I am having the same issue. How can I controll without manually? Is there any patch?

greg-1-anderson commented 2 years ago

Could someone provide a PR with a failing test on Windows?

Ambient-Impact commented 2 years ago

I'm pretty low on bandwidth to help out, but when things settle down I can give it a shot.

weitzman commented 2 years ago

It looks like we currently disable Windows testing of this function https://github.com/consolidation/site-process/blob/main/tests/SiteProcessTest.php#L289

sunlix commented 2 years ago

My tiny obversation on this:

drush drupal:directory outputs "%root": "C:\xampp7.4\htdocs\drupolis_d9/docroot", like this with the decode error. If you disable the line $output = preg_replace('#\\\\{2}#', '\\', $output); it outputs the root path of Drupal like this C:/xampp7.4/htdocs/drupolis_d9/docroot

The / and \ are swapped.

I dont get it, because the preg_replace only should replace the double \\

sunlix commented 2 years ago

If you enable the Windows Test for testSiteProcessJson an run it you can observe that all data will be surrounded by additional double quotes.

'"string"'

will be

'Unable to decode output into JSON: Syntax error\n
\n
""string""'

I just googled an idea to remove the first additional surrounding double quotes: https://stackoverflow.com/a/9734805
and added it to Line 158 in ProcessBase.php

        if (Escape::isWindows()) {
            $output = preg_replace('~^"?(.*?)"?$~', '$1', $output);
            // Doubled double quotes were converted to \\".
            // Revert to double quote.
            $output = str_replace('\\"', '"', $output);
            // Revert of doubled backslashes.
            $output = preg_replace('#\\\\{2}#', '\\', $output);
        }

With that single line I have 7 of 8 passing tests :D But unfortunally I does not solve the case with running drush drupal:directory

The failing test is the first one:

expected:
'Output is empty.'

actual:
'Unable to decode output into JSON: Syntax error\n
\n
ECHO ist eingeschaltet (ON).'

But I dont think that this is relevant to the JSON Decode. I think we have to add some tests with the windows style backslash path parts.

weitzman commented 2 years ago

I created a Drush PR so we don't use those Windows backslashes in status output (which is what dd calls into). This avoids the problem, which might be good enough for now https://github.com/drush-ops/drush/pull/5049? I suppose this issue should re-enable our test on Windows and fix any failures.

sunlix commented 2 years ago

yeah I think this is a nice avoidance for the baclslashed topic. I remind that there is also a case for double quotes in the update messages. Here is a failed JSON output from the simple_sitemap Drupal module from v3 to v4.

In ProcessBase.php line 173:

  Unable to decode output into JSON: Syntax error

  {
      "0": {
          "simple_sitemap": {
              "8401": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8402": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8403": {
                  "results": {
                      "query": "All variants belonging to the built-in "Defau
  lt hreflang" sitemap type have been converted to entities. Custom sitemap t
  ypes added via plugins will have to be recreated manually. See simple_sitem
  ap.type.default_hreflang.yml. The sitemaps need to be regenerated now.",
                      "success": true
                  },
                  "type": "update"
              },
              "8404": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              }
          }
      },
      "drush_batch_process_finished": true
  }
liquidcms commented 9 months ago

Still getting this with drush 11.6.0 (in Windows); which I think was merged into 11.x months before the 11.6.0 release.

liquidcms commented 9 months ago

Still getting this with drush 12.3.4.

greg-1-anderson commented 9 months ago

Thanks for continuing to ping this issue. Activity shows which issues folks still care about. Might have time to look at this over the holidays, but things have been pretty busy, so no promises. A PR with tests would be very welcome here.

liquidcms commented 9 months ago

Doubt i'll come up with a PR but i'll see about posting the point where it fails. I suspect this is caused by specific contrib modules (as sunlix suggests) and the current fix is likely not generic enough to cover all cases.

greg-1-anderson commented 9 months ago

From the simple_sitemap example above, it seems that better encoding would solve one large subset of the problems being encountered here.

greg-1-anderson commented 9 months ago

Although maybe the simple_sitemap example might be an encoding problem in the simple_sitemap module, which would have to be fixed there, not here.

sunlix commented 9 months ago

The failing simple_sitemap example fails because it has no escaped double quotes sequence for JSON. I don’t remind where the update message is fetched and prepared for the JSON output, but I think this is the error. The updates messages are fetched via Reflection in Drupal Core. Maybe you can guide us to the relevant code if you know anything? :)

greg-1-anderson commented 9 months ago

What is the site-process command you're running that produces the un-escaped output shown above?

sunlix commented 9 months ago

just drush updb -y

liquidcms commented 9 months ago

our output leading to fail:

>  [warning] No configuration objects have been updated.
> [WARNING] [locale] [2023-11-29T16:18:36] No configuration objects have been updated. | uid: 0 | request-urifault/ | refer:  | ip:  127.0.0.1 | link:
>  [warning] No configuration objects have been updated.

In ProcessBase.php line 171:

  Unable to decode output into JSON: Syntax error

  [INFO] [system] [2023-11-29T16:18:06] key module installed. | uid: 0 | request-uri: http://default/ | refer
    127.0.0.1 | link:
  [INFO] [system] [2023-11-29T16:18:10] externalauth module installed. | uid: 0 | request-uri: http://default
  r:  | ip:  127.0.0.1 | link:
  [INFO] [system] [2023-11-29T16:18:15] wxt_ext_config module uninstalled. | uid: 0 | request-uri: http://def
  refer:  | ip:  127.0.0.1 | link:
  [INFO] [system] [2023-11-29T16:18:18] ckeditor4_codemirror module installed. | uid: 0 | request-uri: http:/
  / | refer:  | ip:  127.0.0.1 | link:
  [NOTICE] [update] [2023-11-29T16:18:22] The role Anonymous user has had the following non-existent permissi
  moved: access_unpublished node blog_post. | uid: 0 | request-uri: http://default/ | refer:  | ip:  127.0.0.
  :
  [NOTICE] [update] [2023-11-29T16:18:22] The role Authenticated user has had the following non-existent perm
  ) removed: access_unpublished node blog_post. | uid: 0 | request-uri: http://default/ | refer:  | ip:  127.
  link:
  [INFO] [system] [2023-11-29T16:18:27] webform_jqueryui_datepicker module installed. | uid: 0 | request-uri:
  default/ | refer:  | ip:  127.0.0.1 | link:
  [NOTICE] [locale] [2023-11-29T16:18:36] Translations imported: 3 added, 27 updated, 0 removed. | uid: 0 | r
  ri: http://default/ | refer:  | ip:  127.0.0.1 | link:
  {
      "0": {
          "openid_connect_windows_aad": {
              "9201": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "9202": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "9205": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "9206": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              }
          },
          "openid_connect": {
              "8198": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8199": {
                  "results": {
                      "query": "Installed the new config entity type openid_connect_client.",
                      "success": true
                  },
                  "type": "update"
              },
              "8200": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8201": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8202": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8203": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8204": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8205": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8206": {
                  "results": {
                      "query": "Skipped. The new config entity type openid_connect_client is already installe
                      "success": true
                  },
                  "type": "update"
              },
              "8207": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8208": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8209": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8210": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              }
          },
          "wxt_core": {
              "8500": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8501": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "8502": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              }
          },
          "blazy": {
              "8217": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "added_blazy_media_service": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "post_update"
              },
              "added_formatter_blazy_entity": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "post_update"
              },
              "remove_file_repository_service": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "post_update"
              }
          },
          "config_ignore": {
              "8302": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              }
          },
          "config_split": {
              "8003": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              }
          },
          "redirect": {
              "8109": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              }
          },
          "slick": {
              "8210": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              },
              "changed_slick_admin_service_parameter": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "post_update"
              }
          },
          "translatable_menu_link_uri": {
              "8001": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "update"
              }
          },
          "user": {
              "10000": {
                  "results": {
                      "query": "The roles <em class="placeholder">Anonymous user, Authenticated user</em> hav
  n-existent permissions removed. Check the logs for details.",
                      "success": true
                  },
                  "type": "update"
              }
          },
          "webform": {
              "ckeditor": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "post_update"
              },
              "ckeditor01": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "post_update"
              },
              "confirmation_page_noindex": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "post_update"
              },
              "deprecate_jquery_ui_datepicker": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "post_update"
              },
              "deprecate_location_places": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "post_update"
              },
              "multiple_categories": {
                  "results": {
                      "query": null,
                      "success": true
                  },
                  "type": "post_update"
              }
          }
      },
      "1": {
          "files": {
              "translations://key-8.x-1.17.fr.po": {
                  "project": "key",
                  "langcode": "fr",
                  "version": "8.x-1.17",
                  "type": "local",
                  "filename": "key-8.x-1.17.fr.po",
                  "directory": "translations://",
                  "uri": "translations://key-8.x-1.17.fr.po",
                  "timestamp": 1700505422,
                  "last_checked": 1701292683
              }
          },
          "languages": {
              "translations://key-8.x-1.17.fr.po": "fr"
          },
          "stats": {
              "translations://key-8.x-1.17.fr.po": {
                  "additions": 3,
                  "updates": 27,
                  "deletes": 0,
                  "skips": 0,
                  "strings": [
                      "18",
                      "24",
                      "30",
                      "42",
                      "45",
                      "83",
                      "88",
                      "97",
                      "107",
                      "147",
                      "228",
                      "510",
                      "511",
                      "512",
                      "554",
                      "17441",
                      "812",
                      "899",
                      "1012",
                      "1014",
                      "17442",
                      "9959",
                      "10327",
                      "1448",
                      "10062",
                      "1627",
                      "11377",
                      "5362",
                      "5363",
                      "6800"
                  ],
                  "seek": 1602
              }
          }
      },
      "2": [],
      "3": [],
      "4": [],
      "5": [],
      "drush_batch_process_finished": true
  }