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

feat(postgres): Add URL escaping to database connection string #380

Merged

Conversation

alexbricepalo
Copy link
Contributor

@alexbricepalo alexbricepalo commented Nov 21, 2024

Add url.QueryEscape() to properly handle special characters in PostgreSQL connection parameters. This prevents potential connection issues and improves security when credentials contain special characters.

Changes:

  • Escape username with url.QueryEscape()
  • Escape password with url.QueryEscape()
  • Escape database name with url.QueryEscape()

Example special characters handled:

  • @ in usernames (e.g., [email protected])
  • Special chars in passwords (e.g., *, (, ), @)
  • Special chars in database names

This change ensures the DSN string is properly formatted regardless of the characters present in the connection parameters.

Related to PostgreSQL connection string format:
postgres://username:password@hostname:port/database


Important

Add URL escaping to PostgreSQL connection string in config.go to handle special characters in connection parameters.

  • Behavior:
    • Add url.QueryEscape() to escape special characters in PostgreSQL connection parameters in DSN() in config.go.
    • Handles special characters in username, password, host, and database name.
  • Security:
    • Prevents connection issues and improves security by ensuring DSN string is properly formatted.

This description was created by Ellipsis for 4bf628d. It will automatically update as commits are pushed.

Add url.QueryEscape() to properly handle special characters in PostgreSQL
connection parameters. This prevents potential connection issues and improves
security when credentials contain special characters.

Changes:
- Escape username with url.QueryEscape()
- Escape password with url.QueryEscape()
- Escape hostname with url.QueryEscape()
- Escape database name with url.QueryEscape()

Example special characters handled:
- @ in usernames (e.g., [email protected])
- Special chars in passwords (e.g., *, (, ), @)
- Special chars in database names

This change ensures the DSN string is properly formatted regardless of the
characters present in the connection parameters.

Related to PostgreSQL connection string format:
postgres://username:password@hostname:port/database
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 4bf628d in 47 seconds

More details
  • Looked at 38 lines of code in 1 files
  • Skipped 1 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. src/lib/config/config.go:8
  • Draft comment:
    Spelling and grammar issues in comments:
  • Line 8: "this" should be capitalized to "This".
  • Line 19: "sane" could be replaced with "reasonable" for better clarity.
  • Line 19: "config options. when" should be "config options. When".
  • Line 83: "there" should be capitalized to "There".
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_KQa6YOckpS2f2aSz


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

src/lib/config/config.go Outdated Show resolved Hide resolved
Remove url.QueryEscape for Host

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@alexbricepalo
Copy link
Contributor Author

@danielchalef Is anyone around to review this at some point?

@paul-paliychuk
Copy link
Collaborator

@alexbricepalo Thanks for your contribution

@paul-paliychuk paul-paliychuk merged commit bff3c40 into getzep:main Nov 28, 2024
2 checks passed
@paul-paliychuk
Copy link
Collaborator

Published as part of v1.0.2

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