Skip to content

Fail to upload package when description rendering is invalid. #3980

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

Merged
merged 4 commits into from
May 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ warehouse/static/dist
warehouse/admin/static/dist

tags
*.sw*
59 changes: 59 additions & 0 deletions tests/unit/forklift/test_legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,64 @@ def test_fails_with_invalid_names(self, pyramid_config, db_request, name):
"for more information."
).format(name)

@pytest.mark.parametrize(
("description_content_type", "description", "message"),
[
(
"text/x-rst",
".. invalid-directive::",
"400 The description failed to render for 'text/x-rst'. "
"See /the/help/url/ for more information.",
),
(
"",
".. invalid-directive::",
"400 The description failed to render in the default format "
"of reStructuredText. "
"See /the/help/url/ for more information.",
),
],
)
def test_fails_invalid_render(
self, pyramid_config, db_request, description_content_type, description, message
):
pyramid_config.testing_securitypolicy(userid=1)
user = UserFactory.create()
EmailFactory.create(user=user)
db_request.user = user
db_request.remote_addr = "10.10.10.30"

db_request.POST = MultiDict(
{
"metadata_version": "1.2",
"name": "example",
"version": "1.0",
"filetype": "sdist",
"md5_digest": "a fake md5 digest",
"content": pretend.stub(
filename="example-1.0.tar.gz",
file=io.BytesIO(b"A fake file."),
type="application/tar",
),
"description_content_type": description_content_type,
"description": description,
}
)

db_request.help_url = pretend.call_recorder(lambda **kw: "/the/help/url/")

with pytest.raises(HTTPBadRequest) as excinfo:
legacy.file_upload(db_request)

resp = excinfo.value

assert db_request.help_url.calls == [
pretend.call(_anchor="description-content-type")
]

assert resp.status_code == 400
assert resp.status == message

@pytest.mark.parametrize(
"name",
[
Expand Down Expand Up @@ -1158,6 +1216,7 @@ def test_successful_upload(
"filetype": "sdist",
"pyversion": "source",
"content": content,
"description": "an example description",
}
)
db_request.POST.extend([("classifiers", "Environment :: Other Environment")])
Expand Down
29 changes: 28 additions & 1 deletion warehouse/forklift/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
JournalEntry,
BlacklistedProject,
)
from warehouse.utils import http
from warehouse.utils import http, readme


MAX_FILESIZE = 60 * 1024 * 1024 # 60M
Expand Down Expand Up @@ -891,6 +891,33 @@ def file_upload(request):
),
)

# Uploading should prevent broken rendered descriptions.
if form.description.data:
description_content_type = form.description_content_type.data
if not description_content_type:
description_content_type = "text/x-rst"
rendered = readme.render(
form.description.data, description_content_type, use_fallback=False
)
if rendered is None:
if form.description_content_type.data:
message = (
"The description failed to render "
"for '{description_content_type}'."
).format(description_content_type=description_content_type)
else:
message = (
"The description failed to render "
"in the default format of reStructuredText."
)
raise _exc_with_message(
HTTPBadRequest,
"{message} See {projecthelp} for more information.".format(
message=message,
projecthelp=request.help_url(_anchor="description-content-type"),
),
) from None

try:
canonical_version = packaging.utils.canonicalize_version(form.version.data)
release = (
Expand Down
23 changes: 23 additions & 0 deletions warehouse/templates/pages/help.html
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
{% macro file_name_reuse() %}Why am I getting a "Filename or contents already exists" or "Filename has been previously used" error?{% endmacro %}
{% macro project_name() %}Why isn't my desired project name available?{% endmacro %}
{% macro project_name_claim() %}How do I claim an abandoned or previously registered project name?{% endmacro %}
{% macro description_content_type() %}How can I upload a description in a different format?{% endmacro %}
{% macro new_classifier() %}How do I request a new trove classifier?{% endmacro %}
{% macro feedback() %}Where can I report a bug or provide feedback?{% endmacro %}
{% macro maintainers() %}Who maintains PyPI?{% endmacro %}
Expand Down Expand Up @@ -84,6 +85,7 @@ <h2><a href="#problems">Problems</a></h2>
<li><a href="#private-indices">{{ private_indices() }}</a></li>
<li><a href="#project-name">{{ project_name() }}</a></li>
<li><a href="#project-name-claim">{{ project_name_claim() }}</a></li>
<li><a href="#description-content-type">{{ description_content_type() }}</a></li>
<li><a href="#file-size-limit">{{ file_size_limit() }}</a></li>
<li><a href="#admin-intervention">{{ admin_intervention() }}</a></li>
<li><a href="#file-name-reuse">{{ file_name_reuse() }}</a></li>
Expand Down Expand Up @@ -247,6 +249,27 @@ <h3 id="project-name-claim">{{ project_name_claim() }}</h3>
PEP 541 has been accepted, and PyPI is <a href="https://github.com/pypa/warehouse/issues/1506">creating a workflow</a> which will be documented here.
</p>

<h3 id="description-content-type">{{ description_content_type() }}</h3>
<p>
By default,
an upload's description will render
with <a href="http://docutils.sourceforge.net/rst.html">reStructuredText</a>.
If the description is in an alternate format
like Markdown,
a package may set the <tt>long_description_content_type</tt>
in <tt>setup.py</tt>
to the alternate format.
Refer to the <a href="https://packaging.python.org/tutorials/distributing-packages/#description">Python Packaging User Guide</a>
for details on the available formats.
</p>
<p>
PyPI will reject uploads
if the description fails to render.
To check a description locally for validity,
you may use <a href="https://github.com/pypa/readme_renderer">readme_renderer</a>,
which is the same description renderer used by PyPI.
</p>

<h3 id="file-size-limit">{{ file_size_limit() }}</h3>
<p>
If you can't upload your project's release to PyPI because you're hitting the upload file size limit, we can sometimes increase your limit. Make sure you've uploaded at least one release for the project that's <i>under</i> the limit (a <a href="https://www.python.org/dev/peps/pep-0440/#developmental-releases">developmental release version number</a> is fine). Then, <a href="https://github.com/pypa/warehouse/issues/new" target="_blank" rel="noopener">file an issue</a> and tell us:</p>
Expand Down
5 changes: 3 additions & 2 deletions warehouse/utils/readme.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
}


def render(value, content_type=None):
def render(value, content_type=None, use_fallback=True):
if value is None:
return value

Expand All @@ -45,7 +45,8 @@ def render(value, content_type=None):
# If the content was not rendered, we'll render as plaintext instead. The
# reason it's necessary to do this instead of just accepting plaintext is
# that readme_renderer will deal with sanitizing the content.
if rendered is None:
# Skip the fallback option when validating that rendered output is ok.
if use_fallback and rendered is None:
rendered = readme_renderer.txt.render(value)

return rendered
Expand Down