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

fix: don't mistake paths for protocols #6212

Merged
merged 14 commits into from
Nov 28, 2023
10 changes: 3 additions & 7 deletions src/lib/functions/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,15 +290,11 @@ export class FunctionsRegistry {
* function with the given name exists, returns an object with the function
* and the route set to `null`. Otherwise, `undefined` is returned,
*
* @param {string} url
* @param {string} urlPath
* @param {string} method
*/
// @ts-expect-error TS(7006) FIXME: Parameter 'url' implicitly has an 'any' type.
async getFunctionForURLPath(url, method) {
// We're constructing a URL object just so that we can extract the path from
// the incoming URL. It doesn't really matter that we don't have the actual
// local URL with the correct port.
const urlPath = new URL(url, 'http://localhost').pathname
Copy link
Contributor Author

@Skn0tt Skn0tt Nov 27, 2023

Choose a reason for hiding this comment

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

here's an example for why this broke: Screenshot 2023-11-27 at 16 23 40

Copy link
Member

Choose a reason for hiding this comment

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

There is nothing ensuring that urlPath is indeed a URL path and not a full URL, right? I have a feeling that this is precisely why we had this logic here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've played around with it a bit more, and it seems like we had this in place to remove query parameters from the path. 146cb80 implements a fix that still does that

// @ts-expect-error TS(7006) FIXME: Parameter 'urlPath' implicitly has an 'any' type.
async getFunctionForURLPath(urlPath, method) {
const defaultURLMatch = urlPath.match(DEFAULT_FUNCTION_URL_EXPRESSION)

if (defaultURLMatch) {
Expand Down
5 changes: 5 additions & 0 deletions tests/integration/commands/dev/v2-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ describe.runIf(gte(version, '18.13.0'))('v2 api', () => {
expect(await response.text()).toBe(`With literal path: ${url}`)
})

test<FixtureTestContext>('edge case: double slash isnt mistaken as protocol', async ({ devServer }) => {
Skn0tt marked this conversation as resolved.
Show resolved Hide resolved
const response = await fetch(`http://localhost:${devServer.port}//something`)
expect(response.status).toBe(404)
})

test<FixtureTestContext>('doesnt run form logic on paths matching function', async ({ devServer }) => {
const url = `http://localhost:${devServer.port}/products`
await fetch(url, { method: 'POST' })
Expand Down
Loading