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 checkIfUp option #70

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Huntedpix
Copy link
Contributor

Hello !

I've just migrate my project from aemsync 4.0.3 to 5.0.5, and I've notice that the option checkIfUp is always returning an error.

I've search why and found out that fetch doesn't allow to embed credentials into the url.

TypeError: http://admin:admin@localhost:4502/ is an url with embedded credentials.
    at new Request (ui.frontend/node_modules/node-fetch/src/request.js:60:10)

To minimize the change, I've update the code to remove the credential from the target and pass it as headers.

const res = await fetch(target)
let url = target
const headers = new Headers()
const regex = /\/\/(.*:.*)@/gm
Copy link
Owner

Choose a reason for hiding this comment

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

Use new URL() instead of regular expression:

const url = new URL(target)
const auth = `${url.username}:${url.password}`
url.username = ''
url.password = ''
const res = await fetch(url.toString(), {
  headers: { 'Authorization':  `Basic ${btoa(auth) }` }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants