-
Notifications
You must be signed in to change notification settings - Fork 294
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(backend): Preserve url protocol when joining paths #2745
Conversation
🦋 Changeset detectedLatest commit: f0e076d The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Solves #2706 |
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.
🤔 Even though we should apply the change in this PR since it fixes the URL, It seems that we haven't done any change in this part of the code for a lot of time and the current code works for node@v16, node@v18, node@v20.
❯ nvm exec 16 node -e 'console.log(new URL("https:/api.clerk.com/v1/organizations/org_xxxxxxxxxxxxxxxxx/"))'
Running node v16.19.0 (npm v9.2.0)
URL {
href: 'https://api.clerk.com/v1/organizations/org_xxxxxxxxxxxxxxxxx/',
origin: 'https://api.clerk.com/',
protocol: 'https:',
username: '',
password: '',
host: 'api.clerk.com',
hostname: 'api.clerk.com',
port: '',
pathname: '/v1/organizations/org_xxxxxxxxxxxxxxxxx/',
search: '',
searchParams: URLSearchParams {},
hash: ''
}
❯ nvm exec 18 node -e 'console.log(new URL("https:/api.clerk.com/v1/organizations/org_xxxxxxxxxxxxxxxxx/"))'
Running node v18.18.2 (npm v9.8.1)
URL {
href: 'https://api.clerk.com/v1/organizations/org_xxxxxxxxxxxxxxxxx/',
origin: 'https://api.clerk.com/',
protocol: 'https:',
username: '',
password: '',
host: 'api.clerk.com',
hostname: 'api.clerk.com',
port: '',
pathname: '/v1/organizations/org_xxxxxxxxxxxxxxxxx/',
search: '',
searchParams: URLSearchParams {},
hash: ''
}
❯ nvm exec 20 node -e 'console.log(new URL("https:/api.clerk.com/v1/organizations/org_xxxxxxxxxxxxxxxxx/"))'
Running node v20.9.0 (npm v10.1.0)
URL {
href: 'https://api.clerk.com/v1/organizations/org_xxxxxxxxxxxxxxxxx/',
origin: 'https://api.clerk.com/',
protocol: 'https:',
username: '',
password: '',
host: 'api.clerk.com',
hostname: 'api.clerk.com',
port: '',
pathname: '/v1/organizations/org_xxxxxxxxxxxxxxxxx/',
search: '',
searchParams: URLSearchParams {},
hash: ''
}
Yes, this maybe have to do with edge vs node runtime. I'm still investigating. The fix is valid but I will merge after I've successfully reproduced. |
!snapshot |
1 similar comment
!snapshot |
f98c9ae
to
f0e076d
Compare
!snapshot |
Hey @octoper - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i [email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact |
We manage to replicate this in CF workers. It seems like the implementation of the |
(cherry picked from commit c2b9827)
(cherry picked from commit c2b9827) Co-authored-by: panteliselef <[email protected]>
Description
Internally when
joinPaths
was used to join strings that included a url protocol likehttps://
the utility function would remove the duplicate/
from the protocol resulting in a string with this form'https:/api.lclclerk.com/v1/organizations/org_2YRy2Bcrc05EMTY0nMY3qHTjbwj'
When this string was passed in a
URL
constructor likenew URL(...)
the URL would either be fixed automatically or throw an "Invalid URL" error.I believe it came down to node version and different implementations of the URL class under the hood. In any case this PR ensures that absolute url string that is generated from
joinPaths
will keep the protocol intact.Fixes #2706
Checklist
npm test
runs as expected.npm run build
runs as expected.Type of change
Packages affected
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/clerk-expo
@clerk/fastify
gatsby-plugin-clerk
@clerk/localizations
@clerk/nextjs
@clerk/clerk-react
@clerk/remix
@clerk/clerk-sdk-node
@clerk/shared
@clerk/themes
@clerk/types
build/tooling/chore