fix: special characters in the URL's password #317
Conversation
biluohc
changed the title
utils/utils.ts
Outdated
| // Special characters in the password may be url-encoded by URL(), such as = | ||
| try { | ||
| password = decodeURIComponent(password); | ||
| } catch (error) { |
There was a problem hiding this comment.
A ConnectionParamsError must be thrown here if the password encoding fails
There was a problem hiding this comment.
I mean is that even if the decoding fails, the password is not decoded, which is the current behavior. If an error is to be thrown, I am not sure if there will be an abnormality after the change that the current version can run normally.
In fact, if creating a new URL object success, and then url-decoding the password of the url will not fail?
There was a problem hiding this comment.
I found one that failed, probably shouldn't throw an error
> let u = new URL('mysql://root:Mtx%3@loalhost:9999/txdb')
undefined
> u
URL {
href: "mysql://root:Mtx%3@loalhost:9999/txdb",
origin: "null",
protocol: "mysql:",
username: "root",
password: "Mtx%3",
host: "loalhost:9999",
hostname: "loalhost",
port: "9999",
pathname: "/txdb",
hash: "",
search: ""
}
> decodeURIComponent(u.password)
Uncaught URIError: URI malformed
at decodeURIComponent (<anonymous>)
at <anonymous>:2:1
> let password = u.password;
undefined
> password
"Mtx%3"
> try {
password = decodeURIComponent(password);
} catch (error) {}
undefined
> password
"Mtx%3"
There was a problem hiding this comment.
To be honest I'm not sure what to do regarding this. The most "correct" to me seems to ask users to encode the password beforehand, and not leave it up to us to check whether it's encoded or not, but I can see both cases being equally as valid
You know what? Just add a warning similar to
postgres/connection/connection.ts
Line 819 in 235d575
There was a problem hiding this comment.
ok, thanks, I added a console.error
There was a problem hiding this comment.
Let's leave it at
URL-encoded password decode failed
Defaulting to take the URL-encoded password
No other comments, please rebase over latest main because of a faulty commit
There was a problem hiding this comment.
I already rebase over latest main.
utils/utils.ts
Outdated
| // Special characters in the password may be url-encoded by URL(), such as = | ||
| try { | ||
| password = decodeURIComponent(password); | ||
| } catch (error) { |
There was a problem hiding this comment.
To be honest I'm not sure what to do regarding this. The most "correct" to me seems to ask users to encode the password beforehand, and not leave it up to us to check whether it's encoded or not, but I can see both cases being equally as valid
You know what? Just add a warning similar to
postgres/connection/connection.ts
Line 819 in 235d575
No description provided.