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

Author: mblayman

.gitignore

@@ -29,3 +29,4 @@ warehouse/static/dist
 warehouse/admin/static/dist
 
 tags
+*.sw*

tests/unit/forklift/test_legacy.py

@@ -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",
         [
@@ -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")])

warehouse/forklift/legacy.py

@@ -52,7 +52,7 @@
     JournalEntry,
     BlacklistedProject,
 )
-from warehouse.utils import http
+from warehouse.utils import http, readme
 
 
 MAX_FILESIZE = 60 * 1024 * 1024  # 60M
@@ -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 = (

warehouse/templates/pages/help.html

@@ -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 %}
@@ -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>
@@ -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>

warehouse/utils/readme.py

@@ -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
 
@@ -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