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

publishing succeeds when endpoint returns an HTTP 3xx #3600

Closed
3 tasks done
mawillcockson opened this issue Jan 22, 2021 · 4 comments
Closed
3 tasks done

publishing succeeds when endpoint returns an HTTP 3xx #3600

mawillcockson opened this issue Jan 22, 2021 · 4 comments
Labels
kind/bug Something isn't working as expected

Comments

@mawillcockson
Copy link

  • I am on the latest Poetry version.

  • I have searched the issues of this repo and believe that this is not a duplicate.

  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).

  • OS version and name: Debian 10 "buster"

  • Poetry version: 1.1.4 and git

  • Link to a Gist with the contents of your pyproject.toml file: not applicable

tl;dr

a screenshot of a terminal showing the result of running the code snippet below, which is that twine fails and poetry appears to succeed

#!/bin/sh
set -eux
poetry new temp
cd temp && poetry build

export TWINE_USERNAME="invalid username"
export TWINE_PASSWORD="invalid password"
# Note the slash missing from the end of the url
export TWINE_REPOSITORY_URL="https://test.pypi.org/legacy"
set +e
# twine raises an error as expected
twine upload --non-interactive dist/*
set -e

export POETRY_HTTP_BASIC_TEMP_USERNAME="${TWINE_USERNAME}"
export POETRY_HTTP_BASIC_TEMP_PASSWORD="${TWINE_PASSWORD}"
export POETRY_REPOSITORIES_TEMP_URL="${TWINE_REPOSITORY_URL}"
# poetry does not raise an error
poetry publish --repository temp --no-interaction -vvv

# Adding the slash causes poetry to return an expected error
export POETRY_REPOSITORIES_TEMP_URL="${POETRY_REPOSITORIES_TEMP_URL}/"
poetry publish --repository temp --no-interaction -vvv

Issue

I was trying to use poetry to upload packages to the PyPI Test instance, and couldn't figure out how poetry could report a successful publish without my packages showing up on test.pypi.org anywhere.

I was missing a / character at the end of the URL, and PyPI was returning an HTTP 308 redirect to the same URL, with the / at the end:

https://test.pypi.org/legacy 🠒 https://test.pypi.org/legacy/

This bug is reproducible with test URLs like https://httpbin.org/status/399

This may be related to #3198.

I'm very sorry, I haven't dug into this.

Solution / Workaround

poetry behaves as expected when the repository url is a valid PyPI Legacy Upload API endpoint.

In short, I was missing a / character at the end of the URL.

@mawillcockson mawillcockson added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Jan 22, 2021
@mawillcockson
Copy link
Author

It appears that I didn't search hard enough. Some context for the current state of this long standing issue.

Context

As I am posting this comment, the current released version of poetry is 1.1.4, and the current commit that pip will install if the following command is run is a106cb2 (Note: this requires a recent version of pip that has PEP 517 support):

python -m pip install "git+https://github.com/python-poetry/poetry.git"

Previous issues

This bug is mentioned in at least these issues:

It's indirectly mentioned in:

Response to those issues

This comment on #858 mentions that the issue was fixed in the pull request #2285.

This comment on #239 mentions that the issue was fixed in the commit 9035140, which is probably referencing this specific area of changes in that commit.

This issue is addressed by pull request #1310, which was canceled in favor of the pull request #1342, which contains comments from some of the maintainers:

This comment on #742 again points to commit 9035140.

Intended fixes

As seen in the duscussion on #1342, the correct behaviour would be for poetry to raise an error.

Explanation of intended fixes

Pull request #1342 implemented a change to allow poetry to follow HTTP 3xx redirect codes during package upload. In contrast, the official PyPI tool twine returns an error. This is also appears to how poetry is intended to respond.

The reasoning behind returning an error as opposed to automatically following the redirect is two-fold:

First, this comment on pypa/twine#92 mentions that the requests library, which poetry also uses, can be told to automatically follow redirects when submitting an HTTP POST request. Unfortunately, when it does, it does not resubmit the POST body. This comment on #1342 also mentions that requests instead submits a GET request.

This is the area in the source code for twine that also mentions this issue.

This problem with requests' behaviour can be addressed in the way #1342 mentions: First issuing a HEAD request for every upload, and checking if a redirect is issued, and repeating that until a 200 response is received, causing the full POST request to be issued.

Secondly, following a redirect would currently mean resubmitting authentication data and one or more package distribution files to a new URL. If this redirect were issued by a malicious entity, that would allow them to capture this data, without the person running the poetry publish command being able to control this behaviour.

This is mentioned in this area of twine's codebase.

My thoughts

I agree that raising a detailed and helpful error is the most correct thing to do, both in the interest of security, and in the interest of helping raise awareness as soon as possible of something that could be a bug in configuration.

I also think that the current behaviour of both poetry and twine could be improved by starting every upload with a HEAD request to check if the following POST request will be accepted. This would add extra traffic to the majority of uploads, but that is negligible. It would also allow for more correct and safe behaviour for both tools.

Finally, I agree with the author of #1342 that the URLs in the documentation on specifying repositories should be updated to use a test.pypi.org/legacy/.

Closing

This is a duplicate of #858, #239, and #742.

@mawillcockson
Copy link
Author

To be clear, this bug still exists as demonstrated, both in 1.1.4 and a106cb2.

@abn abn removed the status/triage This issue needs to be triaged label Mar 3, 2022
@davnat
Copy link

davnat commented Mar 30, 2022

I think this bug still exists in Poetry v1.1.13.

My use case is:

  • we have a private PyPI repo implemented using the pypiserver docker image behind nginix
  • until yesterday the repo was HTTP only, so all our packages have http://... in poetry.toml
  • yesterday we added SSL support, and configured Nginx to redirect all HTTP/80 connections to HTTPS/443

Now poetry publish apparently succeeds, but the packages are not uploaded, unless we change to https://... in all our packages.

Please note that both output and return status of poetry publish are exactly the same, if the package is uploaded or not.

Copy link

github-actions bot commented Mar 2, 2024

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

No branches or pull requests

3 participants