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

Random failure in "Field translations tests": The configuration file "language.settings" could not be read. #511

Closed
quicksketch opened this issue Dec 29, 2014 · 11 comments

Comments

@quicksketch
Copy link
Member

When running tests, we're currently getting this report fairly frequently from Travis CI, about once every 4-5 runs:

Field API: Field translations tests (FieldTranslationsTestCase) 77 passes, 0 fails, 1 exception, and 4 debug messages
...
ConfigStorageReadException: The configuration file "language.settings" could not be read. in ConfigFileStorage->read() (line 898 of /home/travis/build/backdrop/backdrop/core/includes/config.inc).

This doesn't seem to happen locally running tests through the UI. Command-line tests have not yet been confirmed nor disproven. This is a spin-off from the PR at backdrop/backdrop#612

@lolandese
Copy link

I had the The configuration file "flickr.settings" could not be read. error probably due to a syntax error in my flickr.settings.json file on install. Even correcting it with any sequence of uninstall/install didn't help until I restarted the server with:
sudo service apache2 restart

That seems to have done the trick. It looks that it was a code error that leaves some traces even after correcting it.

@quicksketch
Copy link
Member Author

Although I think we already attempt to clear the PHP file cache, perhaps we're not getting it in all situations where config is updated: http://php.net/manual/en/function.clearstatcache.php

@quicksketch
Copy link
Member Author

After a recent fix to the error reporting, the error now accurately states the real problem:

The configuration file "language.settings" is not properly formatted JSON.

I found a similar error today in UserAdminSettingsFormTest:

---- User: User admin settings (UserAdminSettingsFormTest) ----

Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Exception Uncaught e config.inc         898 ConfigFileStorage->read()          
    ConfigStorageReadException: The configuration file "user.mail" is
    not properly formatted JSON. in ConfigFileStorage->read() (line 898 of
    /home/travis/build/backdrop/backdrop/core/includes/config.inc).

The cause of this seems to be that these tests use $this->randomString() to generate test data. What seems to be happening is that when this data is written and then read back out again, it somehow is invalid JSON. It may be some special character like a back-tick or quote that is causing the issue.

@quicksketch
Copy link
Member Author

I think I've got the solution here. The problem is caused by our PHP 5.3 compatibility for unescaped unicode characters.

We're using a solution such as proposed at http://stackoverflow.com/questions/2934563/how-to-decode-unicode-escape-sequences-like-u00ed-to-proper-utf-8-encoded-cha for unescaping unicode characters. However, the problem we're having is that character combinations that are not intended to be considered Unicode characters are getting unescaped. For example, the string \u1234 is getting written to JSON files as \ሴ. This backslash followed by an unusual character caused the JSON to be invalid on read.

So the solution is pretty simple. We just need to check for a backlash preceding a u character before unescaping the value. I'll file a PR shortly.

For testing this problem, I set up a script to use random characters (same as BackdropWebTestCase::randomString()), then iterated a bunch of config writes and reads until a problem occurred:

define('BACKDROP_ROOT', getcwd());
require_once BACKDROP_ROOT . '/core/includes/bootstrap.inc';
backdrop_bootstrap(BACKDROP_BOOTSTRAP_FULL);

function my_random_string($length = 8) {
  $str = '';
  $character_string = '';
  for ($i = 0; $i < $length; $i++) {
    $character = mt_rand(32, 126);
    $str .= chr($character);
    $character_string .= $character . ' ';
  }
  return $character_string . $str;
}

for ($n = 0; $n < 100000; $n++) {
  $value = my_random_string(20);
  $config = config('test.settings');
  $config->set('key', $value);
  $config->save();

  $config = config('test.settings');
  $config->get('key');
}

Once an invalid string was encountered, Backdrop would throw an exception and stop. Some times I had to run it a few times before such an error occurred.

@docwilmot
Copy link
Contributor

🆒

Very clever!

@quicksketch
Copy link
Member Author

PR at backdrop/backdrop#1164.

This got a little messier than I hoped because checking for a preceding backslash before a Unicode character may match another Unicode character before the current one, so the replacement has to be done twice.

+  $json = preg_replace_callback($pattern, $decode_callback, $json);
+  // Replace a second time to catch any sequences multiple Unicode characters.
+  return preg_replace_callback($pattern, $decode_callback, $json);

Not a deal breaker but just not very elegant.

I also expanded test coverage, as we have been bitten by impartial test coverage in this instance. There was even a line in the tests:

 -    // @todo: Add tests for non-ASCII characters and Unicode.

Heh. Well now we've both fixed support for Unicode characters and added the missing tests.

@quicksketch
Copy link
Member Author

Merged backdrop/backdrop#1164 into 1.x and 1.2.x. It'll be so nice to have a lot fewer random test failures.

@Gormartsen
Copy link
Member

Fail report for this issue:

ERROR: exception 'ConfigStorageReadException' with message 'The configuration file "language.settings" is not properly formatted JSON.

Contents:

{
    "_config_name": "language.settings",
    "_config_static": true,
    "language_negotiation": {
        "language": [
            "locale-url",
            "language-default"
        ]
    },
    "languages": {
        "l1": {
            "langcode": "l1",
            "name": "sm5DS.(\\","enabled":true,"direction":0,"weight":0},"en":{"langcode":"en","name":"English","direction":0,"enabled":true,"weight":0},"l0":{"langcode":"l0","name":"{
                EegdM$t","enabled":true,"direction":0,"weight":0}}}
' in /home/test/github/pr/1441/core/includes/config.inc:900
Stack trace:
#0 /home/test/github/pr/1441/core/includes/config.inc(614): ConfigFileStorage->read('language.settin...')
#1 /home/test/github/pr/1441/core/includes/config.inc(39): Config->load()
#2 /home/test/github/pr/1441/core/modules/language/language.module(111): config('language.settin...')
#3 /home/test/github/pr/1441/core/modules/field/tests/field.test(2836): language_save(Object(stdClass))
#4 /home/test/github/pr/1441/core/modules/simpletest/backdrop_web_test_case.php(597): FieldTranslationsTestCase->setUp()
#5 /home/test/github/pr/1441/core/scripts/run-tests.sh(546): BackdropTestCase->run()
#6 /home/test/github/pr/1441/core/scripts/run-tests.sh(93): simpletest_script_run_one_test('1', 'FieldTranslatio...')
#7 {main}
ERROR: FATAL ListFieldUITestCase: test runner returned a non-zero error code (1).

@Gormartsen
Copy link
Member

One more here: https://zen.ci/backdrop/backdrop/test/test-php53-backdrop_backdrop_1444-7301

Exception Uncaught e config.inc         900 ConfigFileStorage->read()          
    ConfigStorageReadException: The configuration file "user.mail" is not
    properly formatted JSON.

    Contents:
    {
        "_config_name": "user.mail",
        "cancel_confirm_body":
    "?nd[X_,\\","cancel_confirm_subject":"BJ/M&1ibb>RTbLZAI$ac","password_reset_body":"[
            user: name
        ],
        \n\nArequesttoresetthepasswordforyouraccounthasbeenmadeat[
            site: name

    ].\n\nYoumaynowresetyourpasswordbyclickingthislinkorcopyingandpastingittoyourbrowser:
    \n\n[
            user: one-time-login-url

    ]\n\nThislinkcanonlybeusedonceandwillleadyoutoapagewhereyoucansetyourpassword.Itexpiresafteronedayandnothingwillhappenifitisnotused.\n\n--[

@Gormartsen
Copy link
Member

I did not notice that issue closed.
Don't see any other related issue, so I reopen this one.
@quicksketch please correct me if this reports need to go to another issue.

@Gormartsen Gormartsen reopened this Jun 14, 2016
@jenlampton jenlampton modified the milestones: 1.2.3, 1.12.5 Mar 13, 2019
@klonos klonos modified the milestones: 1.12.5, 1.12.6 Mar 20, 2019
@jenlampton jenlampton modified the milestones: 1.12.6, 1.12.7 Apr 17, 2019
@jenlampton jenlampton removed this from the 1.12.7 milestone May 15, 2019
@jenlampton
Copy link
Member

I'm not sure how an old issue form 2017 ended up in the 1.12.7 milestone but this one looks to have been fixed. @Gormartsen if you are still seeing issues "not properly formatted JSON." can you please open a new issue?

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

No branches or pull requests

6 participants