framework icon indicating copy to clipboard operation
framework copied to clipboard
verified publisher icon flarum

fix: cannot install without db prefix

Open luceos opened this issue 1 year ago • 4 comments

Database prefix and database password can be empty.

Caused by the rewrite in https://github.com/flarum/framework/commit/6f11e044a7c18c14793c7d98e99a83a6ade5ae97, as a consequence some settings of the database config are now incorrectly enforced/required.

Necessity

  • [ ] Has the problem that is being solved here been clearly explained?
  • [ ] If applicable, have various options for solving this problem been considered?
  • [x] For core PRs, does this need to be in core, or could it be in an extension?
  • [x] Are we willing to maintain this for years / potentially forever?

Confirmed

  • [ ] Frontend changes: tested on a local Flarum installation.
  • [ ] Backend changes: tests are green (run composer test).
  • [ ] Core developer confirmed locally this works as intended.
  • [ ] Tests have been added, or are not appropriate here.

Required changes:

  • [ ] Related documentation PR: (Remove if irrelevant)
  • [ ] Related core extension PRs: (Remove if irrelevant)

luceos avatar Jun 17 '24 22:06 luceos

Allowing no prefix makes sense, are we allowing no password for socket connections?

You can use no password, on development and production. Whether it's advisable has to be seen. Regardless of that decision, a user will receive errors based on their credentials and the credentials the database server expects anyway.. But blatantly demanding a password would potentially block developers with a local dev environment where root has no password.

Ref: MYSQL_ALLOW_EMPTY_PASSWORD on https://hub.docker.com/_/mysql for instance.

luceos avatar Jun 18 '24 08:06 luceos

Don't merge yet please. Till the big db ones are merged

SychO9 avatar Jun 18 '24 16:06 SychO9

Ref:

  • https://github.com/flarum/framework/pull/3985
  • https://github.com/flarum/framework/pull/3984

Both merged, will rebase and see if the issue persisted.

luceos avatar Aug 03 '24 14:08 luceos

@SychO9 seems one of your PR's made password already optional, I assume sqlite because that's completely file based. So the only thing this PR now fixes is:

a) that prefixes are optional b) that without a prefix no "preg_replace cannot use null as argument"-error is thrown

PS force pushed because I just made the changes on top of the merged/updated 2.x branch instead of rebasing.

luceos avatar Aug 03 '24 15:08 luceos