refactor: Use new URL for connection string parsing#1127
Open
denghongcai wants to merge 2 commits intoporsager:masterfrom
Open
refactor: Use new URL for connection string parsing#1127denghongcai wants to merge 2 commits intoporsager:masterfrom
denghongcai wants to merge 2 commits intoporsager:masterfrom
Conversation
This commit refactors the connection URL parsing logic in `src/index.js` to use the `new URL()` constructor. This change provides a more robust and standardized way to parse connection strings, addressing several limitations of the previous implementation: - **IPv6 Support:** The new parsing logic correctly handles IPv6 addresses in connection strings. - **Encoded Password Handling:** Usernames and passwords with special characters that have been URL-encoded are now correctly decoded. To achieve this, the `parseUrl` function was rewritten to replace the `postgres://` or `postgresql://` protocol with `http://` before passing the string to the `URL` constructor. This allows the use of the standard URL parsing mechanism for a custom protocol. Additionally, two new helper functions, `parseHost` and `parsePort`, have been introduced to correctly extract host and port information from various formats, including single-host, multi-host, and IPv6 addresses.
The previous implementation passed the entire connection string to the URL constructor, which caused a `TypeError: ERR_INVALID_URL` when the string contained multiple hosts. This change updates the `parseUrl` function to first check for multiple hosts in the connection string. If multiple hosts are found, it extracts the full multi-host string for later use and replaces it with only the first host before passing it to the URL constructor. This ensures that the URL is always valid and prevents the parsing error, while still allowing the application to handle multi-host configurations.
porsager
force-pushed
the
master
branch
2 times, most recently
from
4a0fe34 to
3a43815
Compare
Owner
|
Can you add tests so that we can see where the old logic is failing and also the new logic fixing it? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit refactors the connection URL parsing logic in
src/index.jsto use thenew URL()constructor. This change provides a more robust and standardized way to parse connection strings, addressing several limitations of the previous implementation:To achieve this, the
parseUrlfunction was rewritten to replace thepostgres://orpostgresql://protocol withhttp://before passing the string to theURLconstructor. This allows the use of the standard URL parsing mechanism for a custom protocol.Additionally, two new helper functions,
parseHostandparsePort, have been introduced to correctly extract host and port information from various formats, including single-host, multi-host, and IPv6 addresses.