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

URL-decode URL-style DSN #2504

Merged
merged 2 commits into from Jan 15, 2017
Merged

URL-decode URL-style DSN #2504

merged 2 commits into from Jan 15, 2017

Conversation

c960657
Copy link
Contributor

@c960657 c960657 commented Sep 12, 2016

The URL-style DSN does not support escaping, so if the username/password/database name contains certain characters, the connection options simply cannot be represented this way.

This patch URL-decodes the different parts. It introduces a small BC break for usernames/passwords/database names containing the character %.

@deeky666
Copy link
Member

@Ocramius this looks like a reasonable improvement. The BC question arises of course, especially significant for passwords IMO. What do you think? Shall we accept the slight BC and target this patch for 2.6 only? Also I think this needs documentation.

@Ocramius
Copy link
Member

This is sane, and can be landed in 2.6, but it needs documenting in UPGRADE.md before it can be merged.

@c960657 can you add notes for how an upgrade would be performed, and what the BC break for % characters would be? I'd like to also have the % character as part of the test suite data-providers (additional entry to what you added), so that the expected output can be viewed by users hitting the problem, and further regressions prevented.

@Ocramius Ocramius added this to the 2.6 milestone Jan 14, 2017
@Ocramius Ocramius self-requested a review January 14, 2017 22:49
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes as per #2504 (comment)

@deeky666
Copy link
Member

@c960657 also an entry about URL econding/deconding should make its way into the DBAL documentation.

@c960657
Copy link
Contributor Author

c960657 commented Jan 15, 2017

I have added a test and some upgrade information.

@Ocramius Ocramius self-assigned this Jan 15, 2017
@Ocramius
Copy link
Member

@c960657 awesome, thanks!

@Ocramius Ocramius merged commit bfecbeb into doctrine:master Jan 15, 2017
@c960657 c960657 deleted the urldecode-dsn branch January 16, 2017 07:45
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants