pipenv icon indicating copy to clipboard operation
pipenv copied to clipboard

Single Quotes in Pipfile are not handled correctly in latest versions

Open aemerick opened this issue 2 years ago • 5 comments

Issue description

The latest two releases (2021.11.9 and 2021.11.15) no longer support single quotes around environment variables (in order to handle special characters in the environment variables) in Pipfiles (example here of what did work in previous versions: https://pipenv-fork.readthedocs.io/en/latest/advanced.html#injecting-credentials-into-pipfiles-via-environment-variables)

Based on the error messages, the single quotes get converted into %27 in the strings.

aemerick avatar Nov 15 '21 17:11 aemerick

It is already updated in the latest official doc. Don't read the third-party docs.

frostming avatar Nov 16 '21 15:11 frostming

Thanks @frostming. I see now that this appears to be have been an intentional change in behavior with the latest release.

Copying the doc link here in case anyone else stumbles on this issue (since I'm assuming others are probably affected by this): https://pipenv.pypa.io/en/latest/advanced/#injecting-credentials-into-pipfiles-via-environment-variables

aemerick avatar Nov 16 '21 22:11 aemerick

For reference, it looks like this was previously raised in #4856 and then the documentation was adjusted in 1524315baf9610d1b4732fff265625c47ef6375f to say that the password must be manually percent-encoded.

However, if your password is coming from an environment variable, it is entirely possible that it is also used outside of this usecase, where the percent encoding may not be wanted/allowed. Additionally, the quotation advice was previously listed in the documentation as best practice, which means dropping support for it is a BC break.

At the very least, this change should be listed more prominently, especially because the error message it causes does not in any way indicate where the problem is coming from.

But beyond that, I feel that a solution which percent encodes automatically would be better. (I presume that is what was being done before?)

To be clear: this worked in v2021.5.29 and stopped working in v2021.11.5.

The error message manifests as:

[pipenv.exceptions.InstallError]: WARNING: 401 Error, Credentials not correct for ...

GPHemsley-RELX avatar Nov 16 '21 23:11 GPHemsley-RELX

@GPHemsley-RELX Can you comment if this is still an issue on the latest version(s) of pipenv? I believe on the current version we may actually require the format of environment variable preferred by pip, such as: ${MY_ENVAR} and not $MY_ENVAR %MY_ENVAR% though I am not 100% sure on this, but if I right then the documentation is wrong. I'd also like to understand why the documentation recommends If your credentials contain special characters, make sure they are URL-encoded as specified in [rfc3986](https://datatracker.ietf.org/doc/html/rfc3986). -- I don't quite understand how that is supposed to work.

matteius avatar Sep 03 '22 12:09 matteius

@matteius I'm not sure I understand what you're saying. The issue here is how to deal with a password that contains special characters that, when the environment variable is expanded, could interfere with parsing the URL.

Previously, the practice was to put single quotes around it, with any necessary magic being taken care of by pipenv. Then it was changed to require the special characters to be pre-escaped in the environment variable.

So, for example, if your password was foo@bar, in v2021.5.29 you could do this:

PYPI_PASSWORD=foo@bar
[[source]]
url = "https://matteius:'${PYPI_PASSWORD}'@example.com/pypi"

But in v2021.11.5 you have to manually do the url-escaping yourself:

PYPI_PASSWORD=foo%40bar
[[source]]
url = "https://matteius:${PYPI_PASSWORD}@example.com/pypi"

Which means you can't use PYPI_PASSWORD anywhere that expects a literal (un-url-escaped) password.

I have not confirmed whether anything has changed since then, as I believe we instead changed our password to not contain special characters.

GPHemsley-RELX avatar Sep 13 '22 15:09 GPHemsley-RELX