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

CIVI_DB_ARGS, etc is empty but CIVI_DB_DSN has string #107

Open
herbdool opened this issue Dec 8, 2021 · 3 comments
Open

CIVI_DB_ARGS, etc is empty but CIVI_DB_DSN has string #107

herbdool opened this issue Dec 8, 2021 · 3 comments

Comments

@herbdool
Copy link

herbdool commented Dec 8, 2021

This is happening in version v0.3.10 and from the most recent build (I downloaded the repo and ran ./build.sh and found cv.phar created in ./bin - which is undocumented by the way) :

  ["CIVI_DB_ARGS"]=>
  string(0) ""
  ["CIVI_DB_DSN"]=>
  string(89) "mysql://civicrm:civicrm@localhost:3306/civicrm?new_link=true"
  ["CIVI_DB_HOST"]=>
  string(0) ""
  ["CIVI_DB_NAME"]=>
  string(0) ""
  ["CIVI_DB_PASS"]=>
  string(0) ""
  ["CIVI_DB_PORT"]=>
  string(0) ""
  ["CIVI_DB_USER"]=>

This was working not too long ago.

@herbdool
Copy link
Author

@totten I only got around to investigating now. In

$url = parse_url($dsn);
it is using parse_url(). But it can't handle some characters in the password that are otherwise fine when set in civicrm.settings.php. In our case, we've just changed the passwords to exclude characters like: # (they already exclude @, : and some others.

@totten
Copy link
Member

totten commented Mar 12, 2024

@herbdool Oh, wow. I see. So there are slight differences between parse_url() and DB::parseDSN()?

I guess the objective should be RFC compliance for both. RFC 1738: section 2.2 calls out # as generally "unsafe", so translating # => %23 would be canonical. But parsers could differ in how "forgiving" they are about unsafe values. And parsers can also be buggy.

I wanted to understand their behavior better. Quick comparison script:

// Run with "cv scr this-file.php"
$exs = [
  'mysql://user:pass%[email protected]/foo',
  'mysql://user:pass#[email protected]/foo',
];

foreach ($exs as $ex) {
  print_r([
    'raw' => $ex,
    'parse_url' => parse_url($ex),
    'DB::parseDSN' => DB::parseDSN($ex),
  ]);
}

Which gives results:

Scenario parse_url() DB::parseDSN()
user:pass%23word (RFC conformant) OK, but you need an extra call to urldecode() OK
user:pass#word (non-conformant) Fails completely OK

IMHO, it's probably fair for parse_url() and SiteConfigReader to reject literal #, but it's also buggy because it neglects to decode %23.

Is it safe to fix the bug?... I'm not certain. Probably?

@herbdool
Copy link
Author

Are you talking about a bug in cv or in core? Or both? From what I understand DB:parseDSN should be stricter and reject more characters, and cv needs to include a urldecode()?

In Civi\Setup\DbUtil::parseDsn() I see this: $parsed = array_map('urldecode', parse_url($dsn)); but you'd probably want the whole method to use in cv.

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

No branches or pull requests

2 participants