Fix handling of forward slashes and url encoding in credentials #1911

lubert · 2020-01-17T17:16:43Z

Pull Request Check List

This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once, it will save you unnecessary review cycles!

Added tests for changed code.
Updated documentation for changed code.

Note: If your Pull Request introduces a new feature or changes the current behavior, it should be based
on the develop branch. If it's a bug fix or only a documentation update, it should be based on the master branch.

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

Problem

Credentials containing / do not urlencode it, since quote() passes the safe='/' by default

Solution

Pass safe='' so that / is encoded properly

k4nar

Regarding the encoding of /, I think this changed in Python 3.5 with the addition of the quote_via parameter. With which version of Python did you have the issue?

I don't think Poetry should unquote the param, because there is always the chance that a genuine credential contains a string corresponding to an urlencoded char.

lubert · 2020-01-20T17:45:55Z

Hi @k4nar, I had the issue with Python 3.7.1.

The documentation for 2 and 3 list safe='/' as a default argument, which I believe is why / isn't encoded.
https://docs.python.org/3/library/urllib.parse.html#urllib.parse.quote
https://docs.python.org/2/library/urllib.html#urllib.quote
Though I do see quote_via as a param for urlencode

That's a fair point about genuine creds containing a urlencoded string, I was trying to solve for an already encoded password passed via env, which is what we have at work in our build pipeline and is nice for pip since it doesn't do any encoding/unencoding.
I'll update the PR, since we can always decode before passing it to poetry config.

k4nar · 2020-01-20T18:51:30Z

Ah yes, sorry, I was reading the doc of urlencode instead of quote :) .

lubert · 2020-01-31T19:43:52Z

@sdispater, any chance of getting this merged into the next release?

sdispater · 2020-02-21T15:49:58Z

Thanks!

* Fix Github actions cache issues (#1908) * Fix case of `-f` flag * Make it clearer what options to pass to `--format` * fix (masonry.api): `get_requires_for_build_wheel` must return additional list of requirements for building a package, not listed in `pyproject.toml` and not dependencies for the package itself (#1875) fix (tests): adopted tests * Lazy Keyring intialization for PasswordManager (#1892) * Fix Github Actions cache issues (#1928) * Avoid nested quantifiers with overlapping character space on git url parsing (#1902 (#1913) * fix (git): match for `\w` instead of `.` for getting user * change (vcs.git): hold pattern of the regex parts in a dictionary to be consistent over all regexs * new (vcs.git): test for `parse_url` and some fixes for the regex pattern * new (vcs.git): test for `parse_url` with string that should fail * fix (test.vcs.git): make flake8 happy * fix: correct parsing of wheel version with regex. (#1932) The previous regexp was only taking the first integer of the version number, this presented problems when the major version number reached double digits. Poetry would determine that the version of the dependency is '1', rather than, ie: '14'. This caused failures to solve versions. * Fix errors when using the --help option (#1910) * Fix how repository credentials are retrieved from env vars (#1909) # Conflicts: # poetry/utils/password_manager.py * Fix downloading packages from Simplepypi (#1851) * fix downloading packages from simplepypi * unused code removed * remove unused imports * Upgrade dependencies for the 1.0.3 release (#1965) * Bump version to 1.0.3 (#1966) * Fix non-compliant Git URL matching RFC 3986 § 2.3 permits more characters in a URL than were matched. This corrects that, though there may be other deficiencies. This was a regression from v1.0.2, where at least “.” was matched without error. * Update README.md "Updating Poetry" Currently the note in "Updating Poetry" is different from the one below in "Enable tab completion for Bash, Fish, or Zsh". This MR is to make them more consistent. * init: change dev dependency prompt * Fix CI issues (#2069) * fix (setup_reader): check if `func.value` has attr `id` (#2041) * fix(git): get commit sha of git commit from annotated tags (#1948) * fix(git): have annotated tags resolve to the commit sha * fix(git): fix quote * fix(git): change to rev-parse * fix: use correct badge on README (#2065) * Fix #1791: Load repository URL from config (#2061) * Fix #1791: Load repository URL from config * Ran black to fix linting errors * Add test for repo URL env variable * Changed schema to support url in multi dependencies (#2035) * Fix handling of forward slashes and url encoding in credentials (#1911) * Add support for forward slashes and url encoding in credentials * Remove extra newline * Remove unquote * Bump actions/checkout from v1 to v2 (#2075) * Update release.yml * Update main.yml * Fix vendor package as installed package (#1883) (#1981) * Fix vendor package as installed package (#1883) * import from Co-Authored-By: Sébastien Eustace <sebastien.eustace@gmail.com> * test vendor package as installed * refactor * remove blank line Co-authored-by: Sébastien Eustace <sebastien.eustace@gmail.com> * fix(utils.env): import cli_run from virtualenv (#2096) * fix(utils.env): import cli_run from virtualenv if create_environment import failes * fix (utils.env): added accidentally removed code * list .venv when it exists (#1762) * list .venv when it exists * only list when in-project is true * missing config * move logic to manager.list * Add .venv when it exists * fix: exclude subpackage from `setup.py` if `__init__.py` is excluded (#1009) (#1626) * fix: exclude subpackage from `setup.py` if `__init__.py` is excluded Fixes: #1009 * fix: added missing test data * fix: lint test data * change (sdist.git): exclude folders with no python file * fix (sdist.git): make black happy * get_vcs starts searching git folder from tmp dir instead of project (#1946) (#1947) * fix (builder): take `self._original_path` if available to find `.git` folder * change (vcs): use `git rev-parse --show-toplevel` to find git root folder * fix (vcs): change back to original working dir after finding vcs * change (builder): introduce self._original_path to keep original path if(vcs): resolve directory for `get_vcs` * Normalize author name unicode before matching (#2006) * Fix accented characters not being matched in author name Fixes #2004 * Normalized the strings instead of modifying the pattern * Applied isort & black * Fix the url used for installation when fallbacking on PyPI (#2099) * Upgrade dependencies before the 1.0.4 release (#2100) * Upgrade dependencies before the 1.0.4 release (#2103) * Release 1.0.4 (#2101) * Update release script * Bump version to 1.0.4 * Fix release script (#2104) * Fix VCS when git is not in PATH * Upgrade dependencies before the 1.0.5 release (#2111) * Bump version to 1.0.5 (#2112) * Fix GitHub URL for black Black is now officially supported by the Python Software Foundation * Update Contributing.md* Fix markdown formatting* Update link to official website FAQ * Update managing-environments.md Co-authored-by: brandonaut <brandon@hubermx.com> Co-authored-by: finswimmer <finswimmer77@gmail.com> Co-authored-by: Yannick PÉROUX <yannick.peroux@gmail.com> Co-authored-by: Edward George <edwardgeorge@gmail.com> Co-authored-by: Jan Škoda <skoda@jskoda.cz> Co-authored-by: Andrew Marshall <andrew@johnandrewmarshall.com> Co-authored-by: Andrew Selzer <andrewfselzer@gmail.com> Co-authored-by: Andriy Maletsky <andriy.maletsky@gmail.com> Co-authored-by: Julien Lhermitte <705366+jrmlhermitte@users.noreply.github.com> Co-authored-by: Michael Aquilina <michaelaquilina@gmail.com> Co-authored-by: Joshua Cannon <joshdcannon@gmail.com> Co-authored-by: László Velinszky <laszlo.velinszky@meltwater.com> Co-authored-by: Lu Zhu <misterzhu@gmail.com> Co-authored-by: BSKY <git@bsky.moe> Co-authored-by: Trim21 <github@trim21.me> Co-authored-by: Frost Ming <frostming@tencent.com> Co-authored-by: Raphael Yancey <raphael@badfile.net> Co-authored-by: adisbladis <adisbladis@gmail.com> Co-authored-by: Dimitri Merejkowsky <dimitri.merejkowsky@tanker.io> Co-authored-by: Jules Chéron <jules.cheron@gmail.com> Co-authored-by: Alex Povel <48824213+alexpovel@users.noreply.github.com>

github-actions · 2024-03-01T11:07:00Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Add support for forward slashes and url encoding in credentials

c8e9eb8

lubert requested a review from sdispater January 17, 2020 17:16

Remove extra newline

688477b

lubert changed the title ~~Add support for forward slashes and url encoding in credentials~~ Fix handling of forward slashes and url encoding in credentials Jan 17, 2020

k4nar reviewed Jan 20, 2020

View reviewed changes

lubert added 2 commits January 20, 2020 09:57

Remove unquote

b4c7a55

Merge branch 'master' of github.com:lubert/poetry

2722c11

k4nar approved these changes Jan 20, 2020

View reviewed changes

sdispater approved these changes Feb 21, 2020

View reviewed changes

sdispater merged commit 96191ca into python-poetry:master Feb 21, 2020

github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix handling of forward slashes and url encoding in credentials #1911

Fix handling of forward slashes and url encoding in credentials #1911

lubert commented Jan 17, 2020 •

edited

Loading

k4nar left a comment

lubert commented Jan 20, 2020

k4nar commented Jan 20, 2020

lubert commented Jan 31, 2020

sdispater commented Feb 21, 2020

github-actions bot commented Mar 1, 2024

Fix handling of forward slashes and url encoding in credentials #1911

Fix handling of forward slashes and url encoding in credentials #1911

Conversation

lubert commented Jan 17, 2020 • edited Loading

Pull Request Check List

Problem

Solution

k4nar left a comment

Choose a reason for hiding this comment

lubert commented Jan 20, 2020

k4nar commented Jan 20, 2020

lubert commented Jan 31, 2020

sdispater commented Feb 21, 2020

github-actions bot commented Mar 1, 2024

lubert commented Jan 17, 2020 •

edited

Loading