fix: don't mistake paths for protocols #6212
Conversation
| // 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 |
There was a problem hiding this comment.
here's an example for why this broke: 
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
📊 Benchmark resultsComparing with af800af
|
eduardoboucas
changed the title
src/lib/functions/registry.ts
Outdated
| @@ -297,8 +297,8 @@ export class FunctionsRegistry { | |||
| async getFunctionForURLPath(url, method) { | |||
There was a problem hiding this comment.
The parameter should probably be named to urlPath, as it will now break if it's a fully qualified URL? Alternatively, we can keep it as url and check whether if it's a full URL or a URL path, adjusting accordingly.
Co-authored-by: Eduardo Bouças <[email protected]>
Co-authored-by: Eduardo Bouças <[email protected]>
Found another small bug in our routing logic for v2 functions. When requesting a path like
//foo, our routing logic was mistaking the//to be a protocol separator, and inferredfooto be the hostname instead of the path.This PR fixes that!