Can't run tests if password contains /

('Seriously malformed connection URL provided');
Not sure about this saying. If an exception is thrown is that not already "serious"
This error doesn't really tell me what the problem was. How is a user suppose to know it was '/'?

Tagging for usability for their thoughts also.

There's a #ux channel we can post if this doesn't get traction eventually.

Thanks!

Good point.
However, we are not gonna get more information out of parse_url; there is more info about that and references in #6
The wording itself comes from PHP's manual and ultimately I don't see any options for wording and/or UX enhancements here.

To me this feels like an issue of technical implementation.

Looking at the example strings I feel that relaying on parse_url and failing what is not compatible with a chosen function is something to fix by error messages. It needs annoyingly more work but if I look at the example URLs they would be splitable with something like explode using ":" the one part with "@" and the one with port and db with "/". Some cases you can go around this kind of problems by base64 encoding stuff before passing it as URL and the decoding after splitting. These kind of workarounds probably sound silly and are prone to create other issues but the point is the example cases are manageable. We shouldn't report a valid URL as malformed to user if we just check it against a single function response and not against the definition.

The least thing would be to return "Seriously malformed or unsupported connection URL provided". For debugging purposes of course it would be usable to be told what was submitted and how it was understood (split by the function) and the solution would be more or less obvious.

(This reminds me a bit of all the failed domain transfers where the automation didn't accept autogenerated EEP keys that had "'" in them.)

We talked about this in usability meeting today, but didn't have time to come into a conclusion about final recommendation.

I did however notice another thing:

Does this issue actually affect also /core/modules/sqlite/src/Driver/Database/sqlite/Connection.php where $url is parsed with parse_url() even if the username and password are after that unset?

PGSQL doesn't seem to override the createUrlFromConnectionOptions function.

Sorry for posting a lot of comments while digging deeper. I noticed that Database.php before it calls createConnectionOptionsFromURL in concertDbURLConnectionInfo already does some validation for the URL. For instance a preg_match to check if schema exists in $url and if not, throwing

throw new \InvalidArgumentException("Missing scheme in URL '$url'");

Thus I think the validation for password and other possible issues should happen already there prior to calling createConnectionOptionsFromURL. Then I think we would also cover the SQLite version without duplicating code.

In concertDbURLConnectionInfo() the error messages seem include the $url variable which is in my opinion helpful for the administrator/sitebuilder to fix the issue.

Moving to NW as it doesn't seem the remaining task has been achieved.

@smustgrave:

When you tag an issue for usability review, please make it easy for the usability team to review the issue. I am removing the tag for now. If you still want usability review after updating the issue summary, then you can add the tag again.

In #9, you mentioned the string Seriously malformed connection URL provided. If that is shown to the user, even when running texts, then it needs attention. The issue summary does not include any steps to reproduce, so I am not sure if that is why you asked for usability review.

I can give general advice for error messages: they should be descriptive and actionable. In this case, the "descriptive" part should be an explanation that some characters, although valid for MySQL, are not supported by Drupal. The "actionable" part should suggest using a different password.

I am adding the tag for an issue summary update. Since this issue is a bug report, it should include steps to reproduce. The Proposed resolution should be more specific. For example, if we are adding a comment to settings.php, then what is the proposed text? If one of the Remaining tasks is to finalize the text of an exception message, then include a draft version of the text in the Proposed resolution.