-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
(occ db:convert-type) Add support for UNIX sockets #39242
base: master
Are you sure you want to change the base?
(occ db:convert-type) Add support for UNIX sockets #39242
Conversation
@@ -254,6 +257,16 @@ | |||
if ($input->getOption('port')) { | |||
$connectionParams['port'] = $input->getOption('port'); | |||
} | |||
if (strpos($input->getOption('hostname'), ':') !== false) { | |||
// Host variable may carry a socket (or port) | |||
[$host, $portOrSocket] = explode(':', $input->getOption('hostname'), 2); |
Check notice
Code scanning / Psalm
PossiblyUndefinedArrayOffset Note
if (strpos($input->getOption('hostname'), ':') !== false) { | ||
// Host variable may carry a socket (or port) | ||
[$host, $portOrSocket] = explode(':', $input->getOption('hostname'), 2); | ||
if (ctype_digit($portOrSocket)) { | ||
$connectionParams['port'] = $portOrSocket; // to be consistent with how we handle this elsewhere we accept this too but don't document it | ||
} else { | ||
$connectionParams['unix_socket'] = $portOrSocket; | ||
} | ||
$connectionParams['host'] = $host; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (strpos($input->getOption('hostname'), ':') !== false) { | |
// Host variable may carry a socket (or port) | |
[$host, $portOrSocket] = explode(':', $input->getOption('hostname'), 2); | |
if (ctype_digit($portOrSocket)) { | |
$connectionParams['port'] = $portOrSocket; // to be consistent with how we handle this elsewhere we accept this too but don't document it | |
} else { | |
$connectionParams['unix_socket'] = $portOrSocket; | |
} | |
$connectionParams['host'] = $host; | |
} | |
if (preg_match('/^(.+)(:(\d+|[^:]+))?$/',$input->getOption('hostname'), $matches)) { | |
$connectionParams['host'] = $matches[1]; | |
if (isset($matches[3])) { | |
if (is_numeric($matches[3])) { | |
$connectionParams['port'] = $matches[3]; | |
} else { | |
$connectionParams['unix_socket'] = $matches[3]; | |
} | |
} | |
} |
Suggestion :
Using regex is most likely better than explode. /^(.+):(\d+|[^:]+)$/
matches a string that starts with one or more characters ((.+))
followed by a colon (:)
and then either a numeric value (\d+)
or one or more non-colon characters ([^:]+)
until the end of the string ($)
.
The regular expression approach allows you to extract the host name and port number or socket path in a single step, which is more concise than using explode()
and ctype_digit()
. Additionally, it allows for more flexibility in the format of the option value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not copy the comments but they are relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks okay though I left an inline suggestion. Do you mind using regex instead?
Fixes nextcloud#31998 Adds support to `occ db:convert-type` to support UNIX socket connections via MySQL/MariaDB. Uses same `dbhost` / `hostname` parameter parsing logic (adapted) as used elsewhere (at least the relevant parts) for consistency. Signed-off-by: Josh Richards <josh.t.richards@gmail.com>
864c17c
to
1ac39e7
Compare
Adds support to
occ db:convert-type
to support UNIX socket connections via MySQL/MariaDB:Uses same
dbhost
/hostname
parameter parsing logic (adapted) as used elsewhere (at least the relevant parts) for consistency.Adds UI support for non-passworded connections.
Resolves: # [Bug]: db:convert-type does not support mysql passwordless unix socket auth #31998
Summary
TODO
Checklist