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

Connection String #1467

Merged
merged 3 commits into from
Aug 7, 2024
Merged

Connection String #1467

merged 3 commits into from
Aug 7, 2024

Conversation

rozza
Copy link
Member

@rozza rozza commented Aug 1, 2024

Don't output the host and port information if the port is invalid. Reduces risk of leaking password information if the password has not been correctly urlencoded.

JAVA-5560

@rozza rozza requested review from a team and jyemin and removed request for a team August 1, 2024 11:28
Don't output the host and port information if the port is invalid.
Reduces risk of leaking password information if the password has not
been correctly urlencoded.

JAVA-5560
@rozza
Copy link
Member Author

rozza commented Aug 1, 2024

Note: In the unlikely case of the password looking like a port eg: within numeric range and ending in a slash mongodb://foo:1234/@hostname the password would be interpreted as the port.

Copy link
Contributor

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

Is there anything similarly dangerous in ConnectionString#parseHosts?

@rozza
Copy link
Member Author

rozza commented Aug 7, 2024

@jyemin I think all is covered. An unescaped @ sign eg: mongodb://foo:bar@mypass@localhost is already handled and errors with:

The connection string contains invalid user information. If the username or password contains a colon (:) or an at-sign (@) then it must be urlencoded

If the user were to accidently escape the @ sign then it would fail with the new error message about the port.

I added a couple of extra regression tests.

@rozza rozza requested a review from jyemin August 7, 2024 11:26
Copy link
Contributor

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

LGTM!

@rozza rozza merged commit da2ad60 into mongodb:master Aug 7, 2024
2 of 3 checks passed
@rozza rozza deleted the JAVA-5560 branch August 7, 2024 16:20
rozza added a commit to rozza/mongo-java-driver that referenced this pull request Aug 7, 2024
Don't output the host and port information if the port is invalid.
Reduces risk of leaking password information if the password has not
been correctly urlencoded.

JAVA-5560
rozza added a commit to rozza/mongo-java-driver that referenced this pull request Aug 7, 2024
Don't output the host and port information if the port is invalid.
Reduces risk of leaking password information if the password has not
been correctly urlencoded.

JAVA-5560
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

Successfully merging this pull request may close these issues.

2 participants