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

install: fix proxy authentication when password contains special character #2664

Merged
merged 1 commit into from
Apr 17, 2021

Conversation

msalettes
Copy link
Contributor

Hello,

When we install sharp module on a machine which is behind a proxy, we have an error ERR! sharp tunneling socket could not be established, statusCode=407 if the password contains special characters.

This problem is because the agent() function returns a passwordwhere special characters are encoded.
To fix this problem, we just need to decode password before to return it.

Open to any suggestions of course.

@coveralls
Copy link

coveralls commented Apr 15, 2021

Coverage Status

Coverage remained the same at 100.0% when pulling 28351a9 on msalettes:master into ed5d753 on lovell:master.

@lovell
Copy link
Owner

lovell commented Apr 16, 2021

Merci Matthieu, this fix makes sense based on the WHATWG spec for userinfo parsing. We should probably also apply the same change to the username too.

It's interesting to note that this bug/feature also exists in the recently-exposed urlToHttpOptions feature of Node.js itself.

https://github.com/nodejs/node/blob/e46c680bf2b211bbd52cf959ca17ee98c7f657f5/lib/internal/url.js#L1308

@msalettes
Copy link
Contributor Author

@lovell : You're welcome. I asked myself the same question about the username but i was not sure there was a use case to use special characters inside username (except for ., -, _ , but those are not encoded) . But you're right, we can do it , it doesn't cost much to do it. I will update my pr to apply the change on username too.

As you said, it's interesting to the bug also exist on node. Maybe i will submit a pr .

@msalettes
Copy link
Contributor Author

One more thing, the CI fails when testing on freebsd . If i understand well, the error is because FreeBSD 13 uses the version 8.10.5 of vips but the current version of sharp requires the version 8.10.6.
Someone has a solution ?

@lovell lovell merged commit 9c10083 into lovell:master Apr 17, 2021
@lovell lovell added this to the v0.28.2 milestone Apr 17, 2021
@lovell
Copy link
Owner

lovell commented Apr 17, 2021

Yes, the FreeBSD failure is due to the very slightly older vips package (I should probably mark it as being allowed to fail).

Thanks again for the PR, this will be in v0.28.2.

lovell added a commit that referenced this pull request Apr 17, 2021
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.

None yet

3 participants