Skip to content

Commit fbdca7d

Browse files
mtlemiliomtlemilio
authoredMar 27, 2024··
wrangler: Try to URL decode hyperdrive connection string components before sending them to API (#5258)
* wrangler: Try to URL decode hyperdrive connection string components before sending them to API Also do the same in miniflare plugin * fixup! wrangler: Try to URL decode hyperdrive connection string components before sending them to API
·
wrangler@4.6.0@cloudflare/chrome-devtools-patches@0.1.1
1 parent 47b325a commit fbdca7d

File tree

6 files changed

+135
-36
lines changed

6 files changed

+135
-36
lines changed
 

‎.changeset/twenty-laws-mix.md

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"miniflare": minor
3+
"wrangler": minor
4+
---
5+
6+
feature: URL decode components of the Hyperdrive config connection string

‎packages/miniflare/src/plugins/hyperdrive/index.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,9 @@ export const HYPERDRIVE_PLUGIN: Plugin<typeof HyperdriveInputOptionsSchema> = {
8181
designator: {
8282
name: `${HYPERDRIVE_PLUGIN_NAME}:${name}`,
8383
},
84-
database,
85-
user: url.username,
86-
password: url.password,
84+
database: decodeURIComponent(database),
85+
user: decodeURIComponent(url.username),
86+
password: decodeURIComponent(url.password),
8787
scheme,
8888
},
8989
};

‎packages/wrangler/e2e/dev.test.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ describe("hyperdrive dev tests", () => {
484484
[[hyperdrive]]
485485
binding = "HYPERDRIVE"
486486
id = "hyperdrive_id"
487-
localConnectionString = "postgresql://user:pass@127.0.0.1:${port}/some_db"
487+
localConnectionString = "postgresql://user%3Aname:%21pass@127.0.0.1:${port}/some_db"
488488
`,
489489
"src/index.ts": dedent`
490490
export default {
@@ -516,8 +516,8 @@ describe("hyperdrive dev tests", () => {
516516
);
517517
const url = new URL(text);
518518
expect(url.pathname).toBe("/some_db");
519-
expect(url.username).toBe("user");
520-
expect(url.password).toBe("pass");
519+
expect(url.username).toBe("user:name");
520+
expect(url.password).toBe("!pass");
521521
expect(url.host).not.toBe("localhost");
522522
});
523523
});

‎packages/wrangler/src/__tests__/hyperdrive.test.ts

+97-4
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,78 @@ describe("hyperdrive commands", () => {
178178
`);
179179
});
180180

181+
it("should handle creating a hyperdrive config if the user is URL encoded", async () => {
182+
mockHyperdriveRequest();
183+
await runWrangler(
184+
"hyperdrive create test123 --connection-string='postgresql://user%3Aname:password@example.com/neondb'"
185+
);
186+
expect(std.out).toMatchInlineSnapshot(`
187+
"🚧 Creating 'test123'
188+
✅ Created new Hyperdrive config
189+
{
190+
\\"id\\": \\"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\\",
191+
\\"name\\": \\"test123\\",
192+
\\"origin\\": {
193+
\\"host\\": \\"example.com\\",
194+
\\"port\\": 5432,
195+
\\"database\\": \\"neondb\\",
196+
\\"user\\": \\"user:name\\"
197+
},
198+
\\"caching\\": {
199+
\\"disabled\\": false
200+
}
201+
}"
202+
`);
203+
});
204+
205+
it("should handle creating a hyperdrive config if the password is URL encoded", async () => {
206+
mockHyperdriveRequest();
207+
await runWrangler(
208+
"hyperdrive create test123 --connection-string='postgresql://test:a%23%3F81n%287@example.com/neondb'"
209+
);
210+
expect(std.out).toMatchInlineSnapshot(`
211+
"🚧 Creating 'test123'
212+
✅ Created new Hyperdrive config
213+
{
214+
\\"id\\": \\"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\\",
215+
\\"name\\": \\"test123\\",
216+
\\"origin\\": {
217+
\\"host\\": \\"example.com\\",
218+
\\"port\\": 5432,
219+
\\"database\\": \\"neondb\\",
220+
\\"user\\": \\"test\\"
221+
},
222+
\\"caching\\": {
223+
\\"disabled\\": false
224+
}
225+
}"
226+
`);
227+
});
228+
229+
it("should handle creating a hyperdrive config if the database name is URL encoded", async () => {
230+
mockHyperdriveRequest();
231+
await runWrangler(
232+
"hyperdrive create test123 --connection-string='postgresql://test:password@example.com/%22weird%22%20dbname'"
233+
);
234+
expect(std.out).toMatchInlineSnapshot(`
235+
"🚧 Creating 'test123'
236+
✅ Created new Hyperdrive config
237+
{
238+
\\"id\\": \\"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\\",
239+
\\"name\\": \\"test123\\",
240+
\\"origin\\": {
241+
\\"host\\": \\"example.com\\",
242+
\\"port\\": 5432,
243+
\\"database\\": \\"/\\"weird/\\" dbname\\",
244+
\\"user\\": \\"test\\"
245+
},
246+
\\"caching\\": {
247+
\\"disabled\\": false
248+
}
249+
}"
250+
`);
251+
});
252+
181253
it("should handle listing configs", async () => {
182254
mockHyperdriveRequest();
183255
await runWrangler("hyperdrive list");
@@ -258,10 +330,7 @@ describe("hyperdrive commands", () => {
258330
259331
"
260332
`);
261-
expect(std.out).toMatchInlineSnapshot(`
262-
"
263-
If you think this is a bug then please create an issue at https://github.com/cloudflare/workers-sdk/issues/new/choose"
264-
`);
333+
expect(std.out).toMatchInlineSnapshot(`""`);
265334
});
266335

267336
it("should handle updating a hyperdrive config's caching settings", async () => {
@@ -290,6 +359,30 @@ describe("hyperdrive commands", () => {
290359
`);
291360
});
292361

362+
it("should handle disabling caching for a hyperdrive config", async () => {
363+
mockHyperdriveRequest();
364+
await runWrangler(
365+
"hyperdrive update xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx --caching-disabled=true"
366+
);
367+
expect(std.out).toMatchInlineSnapshot(`
368+
"🚧 Updating 'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'
369+
✅ Updated xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx Hyperdrive config
370+
{
371+
\\"id\\": \\"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\\",
372+
\\"name\\": \\"test123\\",
373+
\\"origin\\": {
374+
\\"host\\": \\"example.com\\",
375+
\\"port\\": 5432,
376+
\\"database\\": \\"neondb\\",
377+
\\"user\\": \\"test\\"
378+
},
379+
\\"caching\\": {
380+
\\"disabled\\": true
381+
}
382+
}"
383+
`);
384+
});
385+
293386
it("should handle updating a hyperdrive config's name", async () => {
294387
mockHyperdriveRequest();
295388
await runWrangler(

‎packages/wrangler/src/hyperdrive/create.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ export async function handler(
8888
host: url.hostname,
8989
port: parseInt(url.port),
9090
scheme: url.protocol.replace(":", ""),
91-
database: url.pathname.replace("/", ""),
92-
user: url.username,
93-
password: url.password,
91+
database: decodeURIComponent(url.pathname.replace("/", "")),
92+
user: decodeURIComponent(url.username),
93+
password: decodeURIComponent(url.password),
9494
},
9595
caching: {
9696
disabled: args.cachingDisabled,

‎packages/wrangler/src/hyperdrive/update.ts

+23-23
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { readConfig } from "../config";
2+
import { UserError } from "../errors";
23
import { logger } from "../logger";
34
import { patchConfig } from "./client";
45
import { hyperdriveBetaWarning } from "./utils";
@@ -62,12 +63,17 @@ export function options(yargs: CommonYargsArgv) {
6263
}
6364

6465
const requiredOriginOptions = [
65-
"origin-host",
66-
"origin-port",
66+
"originHost",
67+
"originPort",
6768
"database",
68-
"origin-user",
69-
"origin-password",
70-
];
69+
"originUser",
70+
"originPassword",
71+
] as const;
72+
73+
// utility for displaying the yargs options to the user when displaying the "all or nothing" error message
74+
function camelToKebab(str: string): string {
75+
return str.replace(/([a-z0-9])([A-Z])/g, "$1-$2").toLowerCase();
76+
}
7177

7278
function isOptionSet<T extends object>(args: T, key: keyof T): boolean {
7379
return key in args && args[key] !== undefined;
@@ -78,17 +84,17 @@ export async function handler(
7884
) {
7985
// check if all or none of the required origin fields are set, since we don't allow partial updates of the origin
8086
const allOriginFieldsSet = requiredOriginOptions.every((field) =>
81-
isOptionSet(args, field as keyof typeof options)
87+
isOptionSet(args, field)
8288
);
8389
const noOriginFieldSet = requiredOriginOptions.every(
84-
(field) => !isOptionSet(args, field as keyof typeof options)
90+
(field) => !isOptionSet(args, field)
8591
);
8692

8793
if (!allOriginFieldsSet && !noOriginFieldSet) {
88-
throw new Error(
89-
`When updating the origin, all of the following must be set: ${requiredOriginOptions.join(
90-
", "
91-
)}`
94+
throw new UserError(
95+
`When updating the origin, all of the following must be set: ${requiredOriginOptions
96+
.map((option) => camelToKebab(option))
97+
.join(", ")}`
9298
);
9399
}
94100

@@ -104,26 +110,20 @@ export async function handler(
104110

105111
if (allOriginFieldsSet) {
106112
database.origin = {
113+
scheme: args.originScheme ?? "postgresql",
107114
host: args.originHost,
108115
port: args.originPort,
109116
database: args.database,
110117
user: args.originUser,
111118
password: args.originPassword,
112119
};
113-
if (args.originScheme !== undefined) {
114-
database.origin.scheme = args.originScheme;
115-
} else {
116-
database.origin.scheme = "postgresql"; // setting default if not passed
117-
}
118120
}
119121

120-
if (args.cachingDisabled || args.maxAge || args.swr) {
121-
database.caching = {
122-
disabled: args.cachingDisabled,
123-
maxAge: args.maxAge,
124-
staleWhileRevalidate: args.swr,
125-
};
126-
}
122+
database.caching = {
123+
disabled: args.cachingDisabled,
124+
maxAge: args.maxAge,
125+
staleWhileRevalidate: args.swr,
126+
};
127127

128128
const updated = await patchConfig(config, args.id, database);
129129
logger.log(

0 commit comments

Comments
 (0)
Please sign in to comment.