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

Automatically handle seeing a port 3306 in a connection string #144

Open
mattrobenolt opened this issue Oct 18, 2023 · 4 comments
Open

Automatically handle seeing a port 3306 in a connection string #144

mattrobenolt opened this issue Oct 18, 2023 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@mattrobenolt
Copy link
Member

mattrobenolt commented Oct 18, 2023

port 3306 can never work for us, and it's common that a DATABASE_URL that you've used in another environemnt that was a traditional mysql driver looks something like: mysql://user:pass@aws.connect.psdb.cloud:3306/dbname

We already ignore the mysql:// protocol and rewrite it to https://, so we should similarly drop the 3306 if a connection is overly explicit.

Refs #142

@mattrobenolt mattrobenolt added enhancement New feature or request good first issue Good for newcomers labels Oct 18, 2023
@kailash360
Copy link

We can use RegEx to recognize port number in the connection string. Can I take up this issue?

@mattrobenolt
Copy link
Member Author

I don't think we need a regular expression here, we already fully parse the URL, it should have the port already I'd suspect, I haven't looked much.

@kailash360
Copy link

I checked the part where we parse the connection string and it is transformed into an URL object (here). We can use the port property in this URL object to check if it is allowed or not. We can simply create a list of not-allowed ports in a constants file and check if the port that we got from the URL object lies there or not, based on which we can proceed with the connection or throw and error.

@mattrobenolt
Copy link
Member Author

That's more what I'd expect for implementation. I don't think we need to even go that complex though, and I feel we only need to special case 3306 and coerce it to https/443. I don't think there's any reason to maintain anything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants