-
Notifications
You must be signed in to change notification settings - Fork 574
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
fix(wren-ui): encode pwd when building connection url #1281
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request modifies the connection URL construction in the data source file. The password for both the Postgres and Click House data sources is now URL-encoded using encodeURIComponent() before being embedded in the connection URL. This change ensures that any special characters in the password do not disrupt the URL format. No exported or public entity declarations have been altered. Changes
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wren-ui/src/apollo/server/dataSource.ts (1)
171-172
: Add comment for consistency with Postgres implementation.The implementation correctly uses
encodeURIComponent()
, matching the Postgres changes.Add a comment for consistency with the Postgres implementation:
+ // url encode password const encodedPassword = encodeURIComponent(password); let connectionUrl = `clickhouse://${user}:${encodedPassword}@${host}:${port}/${database}?`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wren-ui/src/apollo/server/dataSource.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
wren-ui/src/apollo/server/dataSource.ts (1)
101-104
: LGTM! Verify password encoding with special characters.The implementation correctly uses
encodeURIComponent()
to encode the password in the Postgres connection URL.Please verify that the encoding works correctly with passwords containing special characters like:
- Spaces
- Special characters (@#$%^&*)
- Unicode characters
- URL-specific characters (:/?#[]@)
✅ Verification successful
🌐 Web query:
What characters does encodeURIComponent() encode in JavaScript?
💡 Result:
The `encodeURIComponent()` function in JavaScript encodes all characters except the following **unreserved characters**: `A-Z a-z 0-9 - _ . ! ~ * ' ( )`[3][7][8]. It specifically encodes these **reserved/special characters** used in URI components: ``` , / ? : @ & = + $ # ``` and other unsafe/non-ASCII characters like spaces (`%20`), percent signs (`%25`), quotation marks (`%22`), etc.[1][3][4][5][7]. ### Key Details: 1. **Encodes more aggressively than `encodeURI()`** While `encodeURI()` preserves URI syntax characters (like `/`, `:`, and `@`), `encodeURIComponent()` encodes them[3][6][8]: ```javascript encodeURIComponent("/?:@&=+$#") // → "%2F%3F%3A%40%26%3D%2B%24%23" ``` 2. **Use case** Designed for encoding **URI components** (like query parameters), not full URLs[3][4][6]: ```javascript const param = "price=100$&color=red"; const encoded = encodeURIComponent(param); // → "price%3D100%24%26color%3Dred" ``` 3. **Special character replacements** - Space → `%20` (not `+` like in form encoding)[4][7] - `&` → `%26`[1][5] - `=` → `%3D`[5] - `#` → `%23`[5] ### Comparison Table: encodeURI vs encodeURIComponent | Character | encodeURI | encodeURIComponent | |-----------|-----------|--------------------| | `/` | No | Yes (`%2F`) | | `:` | No | Yes (`%3A`) | | `@` | No | Yes (`%40`) | | `&` | No | Yes (`%26`) | | `=` | No | Yes (`%3D`) | | `+` | No | Yes (`%2B`) | | `$` | No | Yes (`%24`) | | `,` | No | Yes (`%2C`) | Use `encodeURIComponent()` when encoding values for URL components like query parameters or hash fragments[3][4][6]. For complete URLs with intact domain/path structure, use `encodeURI()` instead[3][8]. Citations: - 1: https://www.w3schools.com/jsref/jsref_encodeuricomponent.asp - 2: https://www.boardinfinity.com/blog/encoding-and-decoding-url-in-javascript-how/ - 3: https://www.30secondsofcode.org/js/s/encodeuri-encodeuricomponent/ - 4: https://frontendmasters.com/blog/encoding-and-decoding-urls-in-javascript/ - 5: https://documentation.n-able.com/N-central/userguide/Content/Further_Reading/API_Level_Integration/API_Integration_URLEncoding.html - 6: https://love2dev.com/blog/whats-the-difference-between-encodeuri-and-encodeuricomponent/ - 7: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent... - 8: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURI - 9: http://www.w3schools.com/Jsref/jsref_encodeuri.asp - 10: https://phpgrid.com/blog/when-to-use-encodeuri-and-encodeuricomponent/
Password Encoding Verified!
The code correctly uses
encodeURIComponent()
to encode the password in the connection URL. As confirmed by the web query, this function encodes all reserved/special characters (such as spaces,@
,$
,%
,&
, etc.) except for the unreserved characters. This ensures that passwords with special characters will be handled safely in the connection string.
Description
encode pwd when building connection url
Issue
#1278
Summary by CodeRabbit