Use PEP 503 rules to validate upload filename

tests/unit/forklift/test_legacy.py

@@ -20,7 +20,6 @@
 from cgi import FieldStorage
 from unittest import mock
 
-import pkg_resources
 import pretend
 import pytest
 import requests
@@ -667,6 +666,45 @@ def test_is_duplicate_true(self, pyramid_config, db_request):
 
  assert legacy._is_duplicate_file(db_request.db, filename, hashes)
 
+ @pytest.mark.parametrize("file_name_prefix", ["foo_bar", "foo.bar"])
+ @pytest.mark.parametrize("project_name", ["foo_bar", "foo.bar"])
+ def test_is_duplicate_true_non_normalized_filename(
+ self, pyramid_config, db_request, file_name_prefix, project_name
+ ):
+ pyramid_config.testing_securitypolicy(userid=1)
+
+ user = UserFactory.create()
+ EmailFactory.create(user=user)
+ project = ProjectFactory.create(name=project_name)
+ release = ReleaseFactory.create(project=project, version="1.0")
+ RoleFactory.create(user=user, project=project)
+
+ filename = "{}-{}.tar.gz".format(project_name, release.version)
+ file_content = io.BytesIO(_TAR_GZ_PKG_TESTDATA)
+ file_value = file_content.getvalue()
+
+ hashes = {
+ "sha256": hashlib.sha256(file_value).hexdigest(),
+ "md5": hashlib.md5(file_value).hexdigest(),
+ "blake2_256": hashlib.blake2b(file_value, digest_size=256 // 8).hexdigest(),
+ }
+ db_request.db.add(
+ File(
+ release=release,
+ filename=filename,
+ md5_digest=hashes["md5"],
+ sha256_digest=hashes["sha256"],
+ blake2_256_digest=hashes["blake2_256"],
+ path="source/{name[0]}/{name}/{filename}".format(
+ name=project.name, filename=filename
+ ),
+ )
+ )
+
+ duplicate_filename = "{}-{}.tar.gz".format(file_name_prefix, release.version)
+
+ assert legacy._is_duplicate_file(db_request.db, duplicate_filename, hashes)
+
  def test_is_duplicate_none(self, pyramid_config, db_request):
  pyramid_config.testing_securitypolicy(userid=1)
 
@@ -2354,7 +2392,7 @@ def test_upload_fails_with_diff_filename_same_blake2(
  "filetype": "sdist",
  "md5_digest": hashlib.md5(file_content.getvalue()).hexdigest(),
  "content": pretend.stub(
- filename="{}-fake.tar.gz".format(project.name),
+ filename="{}-0.1.tar.gz".format(project.name),
  file=file_content,
  type="application/tar",
  ),
@@ -2388,7 +2426,14 @@ def test_upload_fails_with_diff_filename_same_blake2(
  "400 File already exists. See /the/help/url/ for more information."
  )
 
- def test_upload_fails_with_wrong_filename(self, pyramid_config, db_request):
+ @pytest.mark.parametrize(
+ "filename",
+ [
+ "nope-{version}.tar.gz",
+ "nope-{version}-py3-none-any.whl",
+ ],
+ )
+ def test_upload_fails_with_wrong_prefix(self, pyramid_config, db_request, filename):
  pyramid_config.testing_securitypolicy(userid=1)
 
  user = UserFactory.create()
@@ -2398,7 +2443,7 @@ def test_upload_fails_with_wrong_filename(self, pyramid_config, db_request):
  release = ReleaseFactory.create(project=project, version="1.0")
  RoleFactory.create(user=user, project=project)
 
- filename = "nope-{}.tar.gz".format(release.version)
+ filename = filename.format(version=release.version)
 
  db_request.POST = MultiDict(
  {
@@ -2422,11 +2467,108 @@ def test_upload_fails_with_wrong_filename(self, pyramid_config, db_request):
 
  assert resp.status_code == 400
  assert resp.status == (
- "400 Start filename for {!r} with {!r}.".format(
- project.name, pkg_resources.safe_name(project.name).lower()
+ "400 Invalid filename: Start filename for project {!r} with {!r}.".format(
+ project.name, project.normalized_name.replace("-", "_")
  )
  )
 
+ def test_wheel_upload_passes_with_period_in_prefix(
+ self, pyramid_config, db_request, metrics, monkeypatch
+ ):
+ """
+ This tests existing behavior which allows periods in the project name,
+ which violates the wheel spec at [1], which requires that the filename
+ start with the PEP 503 normalization of the project name, followed by
+ replacing - with _.
+
+ [1] https://packaging.python.org/specifications/binary-distribution-format/
+ """
+ pyramid_config.testing_securitypolicy(userid=1)
+
+ user = UserFactory.create()
+ db_request.user = user
+ EmailFactory.create(user=user)
+ project = ProjectFactory.create(name="some.project")
+ release = ReleaseFactory.create(project=project, version="1.0")
+ RoleFactory.create(user=user, project=project)
+
+ filename = "{project_name}-{version}-py3-none-any.whl".format(
+ version=release.version, project_name=project.name
+ )
+ monkeypatch.setattr(legacy, "_is_valid_dist_file", lambda *a, **kw: True)
+
+ db_request.find_service = pretend.call_recorder(
+ lambda svc, name=None, context=None: {
+ IFileStorage: pretend.stub(store=lambda *a, **kw: None),
+ IMetricsService: metrics,
+ }.get(svc)
+ )
+
+ db_request.user_agent = "warehouse-tests/6.6.6"
+ db_request.POST = MultiDict(
+ {
+ "metadata_version": "1.2",
+ "name": project.name,
+ "version": release.version,
+ "filetype": "bdist_wheel",
+ "pyversion": "py3",
+ "md5_digest": _TAR_GZ_PKG_MD5,
+ "content": pretend.stub(
+ filename=filename,
+ file=io.BytesIO(_TAR_GZ_PKG_TESTDATA),
+ type="application/tar",
+ ),
+ }
+ )
+
+ resp = legacy.file_upload(db_request)
+
+ assert resp.status_code == 200
+
+ @pytest.mark.parametrize(
+ "filename",
+ [
+ "nope-notaversion.tar.gz",
+ "nope-notaversion-py3-none-any.whl",
+ ],
+ )
+ def test_upload_fails_with_invalid_version(
+ self, pyramid_config, db_request, filename
+ ):
+ pyramid_config.testing_securitypolicy(userid=1)
+
+ user = UserFactory.create()
+ db_request.user = user
+ EmailFactory.create(user=user)
+ project = ProjectFactory.create()
+ release = ReleaseFactory.create(project=project, version="1.0")
+ RoleFactory.create(user=user, project=project)
+
+ filename = filename.format(version=release.version)
+
+ db_request.POST = MultiDict(
+ {
+ "metadata_version": "1.2",
+ "name": project.name,
+ "version": release.version,
+ "filetype": "sdist",
+ "md5_digest": "nope!",
+ "content": pretend.stub(
+ filename=filename,
+ file=io.BytesIO(b"a" * (legacy.MAX_FILESIZE + 1)),
+ type="application/tar",
+ ),
+ }
+ )
+
+ with pytest.raises(HTTPBadRequest) as excinfo:
+ legacy.file_upload(db_request)
+
+ resp = excinfo.value
+
+ assert resp.status_code == 400
+ assert resp.status == "400 Invalid filename: Invalid version: 'notaversion'."
+
  def test_upload_fails_with_invalid_extension(self, pyramid_config, db_request):
  pyramid_config.testing_securitypolicy(userid=1)
 

warehouse/forklift/legacy.py

@@ -26,7 +26,6 @@
 import packaging.specifiers
 import packaging.utils
 import packaging.version
-import pkg_resources
 import requests
 import stdlib_list
 import wtforms
@@ -735,6 +734,15 @@ def _is_valid_dist_file(filename, filetype):
  return True
 
 
+def _parse_filename(filename):
+ if filename.endswith(".whl"):
+ parse_func = packaging.utils.parse_wheel_filename
+ else:
+ parse_func = packaging.utils.parse_sdist_filename
+
+ return parse_func(filename)
+
+
 def _is_duplicate_file(db_session, filename, hashes):
  """
  Check to see if file already exists, and if it's content matches.
@@ -758,7 +766,8 @@ def _is_duplicate_file(db_session, filename, hashes):
 
  if file_ is not None:
  return (
- file_.filename == filename
+ # This has the effect of canonicalizing the project name and version
+ _parse_filename(file_.filename) == _parse_filename(filename)
  and file_.sha256_digest == hashes["sha256"]
  and file_.md5_digest == hashes["md5"]
  and file_.blake2_256_digest == hashes["blake2_256"]
@@ -1192,13 +1201,29 @@ def file_upload(request):
  "for more information.",
  )
 
- # Make sure that our filename matches the project that it is being uploaded
- # to.
- prefix = pkg_resources.safe_name(project.name).lower()
- if not pkg_resources.safe_name(filename).lower().startswith(prefix):
+ try:
+ parsed_filename_parts = _parse_filename(filename)
+ except (
+ packaging.utils.InvalidSdistFilename,
+ packaging.utils.InvalidWheelFilename,
+ packaging.version.InvalidVersion,
+ ) as e:
+ raise _exc_with_message(HTTPBadRequest, f"Invalid filename: {e}.")
+
+ # Make sure that the normalized version of the project name in the filename
+ # matches the normalized project name that it is being uploaded to.
+ #
+ # NB: This allows periods the project name, which violates the wheel spec
+ # at [1], which requires that the filename start with the PEP 503
+ # normalization of the project name, followed by replacing - with _.
+ #
+ # [1] https://packaging.python.org/specifications/binary-distribution-format/
+ if not parsed_filename_parts[0] == project.normalized_name:
  raise _exc_with_message(
  HTTPBadRequest,
- "Start filename for {!r} with {!r}.".format(project.name, prefix),
+ "Invalid filename: Start filename for project {!r} with {!r}.".format(
+ project.name, project.normalized_name.replace("-", "_")
+ ),
  )
 
  # Check the content type of what is being uploaded