Fix DotEnv class turning export to empty string

[paulbalandan]

Description
Fixes #6621

I think the bug is on the incorrect sanitization of exported env variables with the syntax export varname=value.

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[kenjis]

What is the purpose of deleting these strings?

[paulbalandan]

I'm not entirely sure, but I think this was meant to make 'foo' or "foo" to just foo

[kenjis]

Is export varname=value valid syntax for .env?
It seems no such explanation in the user guide.

[paulbalandan]

Is export varname=value valid syntax for .env? It seems no such explanation in the user guide.

I think that was the intention.

[paulbalandan]

Okay. Since there was mention that the code comes mostly from vlucas/phpdotenv, I checked the repo and found this code in v2.0.0
https://github.com/vlucas/phpdotenv/blob/058b9b03b945b6c3eced241935c45822fd2bf4e8/src/Loader.php#L249-L254

It seems our DotEnv class is based on an old code.

[MGatner]

Looks good, an edge case I would like to see covered.

[MGatner]

Just saw your comment - should we use an actual package for this, either via Composer or unlined like we do with Kint? Seems like a high-risk security area to be based off old third-party code.

[paulbalandan]

Just saw your comment - should we use an actual package for this, either via Composer or unlined like we do with Kint? Seems like a high-risk security area to be based off old third-party code.

Well I'm in favor. Less maintenance burden on us.

[kenjis]

What security concern is in DotEnv?
It seems a few methods were borrowed from vlucas/phpdotenv when we see the code comment.

Config is a part that has slowed down considerably with CI4, and I don't want to see it get any heavier.

[kenjis]

It seems vlucas/phpdotenv support export varname=value:
https://github.com/vlucas/phpdotenv/blob/master/tests/fixtures/env/exported.env

But for what?

[MGatner]

I'm good with this fix for now. I do agree that Config has gotten rather bloated, but I think DotEnv is distinct enough (and necessary) that I would be in favor of some research at least about outsourcing it.

[kenjis]

This is not related to this PR, but I've found this PHP extention:
https://github.com/sjinks/php-ext-dotenv

The parser is compatible with that of Node.js dotenv version 10.0 (test cases use the same environment file to test parsing).

[kenjis]

Before: $_ENV

    'char.expo.foo' => string (12) "exported foo"
    'character..var' => string (8) "variable"
    ' char.var' => string (9) "character"
    ' char.s' => string (7) "imports"
    'fruit.' => string (6) "banana"

After: $_ENV

    'char.expo.foo' => string (12) "exported foo"
    'character.export.var' => string (8) "variable"
    'char.var' => string (9) "character"
    'char.exports' => string (7) "imports"
    'fruit.export' => string (6) "banana"

[kenjis]

Node.js dotenv@16.0.3:

  'char.expo.foo': 'exported foo',
  'character.export.var': 'variable',
  'char.var': 'character',
  'char.exports': 'imports',
  'fruit.export': 'banana'

It seems that export is handled the same way in Node dotenv as in this PR.

[kenjis]

As the bug fix, this has no problem.

[paulbalandan]

Thanks for the reviews