Skip to content

Commit a79e516

Browse files
committed
Fix: return 404 instead of 500 for invalid advisory IDs
latest_for_avid() raised DoesNotExist instead of returning None, so the Http404 fallback in AdvisoryDetails, AdvisoryPackagesDetails and AdvisoryPackageCommitPatchDetails was never reached. Fixes #2396
1 parent c006e6c commit a79e516

3 files changed

Lines changed: 62 additions & 1 deletion

File tree

vulnerabilities/models.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2926,7 +2926,10 @@ def to_dict(self):
29262926

29272927
class AdvisoryV2QuerySet(BaseQuerySet):
29282928
def latest_for_avid(self, avid: str):
2929-
return self.get(avid=avid, is_latest=True)
2929+
try:
2930+
return self.get(avid=avid, is_latest=True)
2931+
except self.model.DoesNotExist:
2932+
return None
29302933

29312934
def latest_per_avid(self):
29322935
return self.filter(is_latest=True)
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#
2+
# Copyright (c) nexB Inc. and others. All rights reserved.
3+
# VulnerableCode is a trademark of nexB Inc.
4+
# SPDX-License-Identifier: Apache-2.0
5+
# See http://www.apache.org/licenses/LICENSE-2.0 for the license text.
6+
# See https://github.com/aboutcode-org/vulnerablecode for support or download.
7+
# See https://aboutcode.org for more information about nexB OSS projects.
8+
#
9+
10+
import time
11+
12+
import pytest
13+
from django.urls import reverse
14+
15+
INVALID_AVID = "pysec/PYSEC-3000-0"
16+
17+
18+
@pytest.mark.django_db
19+
@pytest.mark.parametrize(
20+
"url_name",
21+
[
22+
"advisory_details",
23+
"advisory_package_details",
24+
"advisory_package_commit_details",
25+
],
26+
)
27+
def test_advisory_views_return_404_for_invalid_avid(client, url_name):
28+
"""
29+
Requesting an advisory-related page for an avid that does not exist should
30+
return a 404 Page Not Found, not a 500 Server Error.
31+
32+
Regression test for: "Advisory Page Returns 500 Instead of 404 for Invalid
33+
Advisory IDs".
34+
"""
35+
# Satisfy AltchaProtectionMiddleware so the request reaches the view
36+
# instead of being redirected to the captcha page.
37+
session = client.session
38+
session["altcha_verified_at"] = time.time()
39+
session.save()
40+
41+
url = reverse(url_name, kwargs={"avid": INVALID_AVID})
42+
response = client.get(url)
43+
44+
assert response.status_code == 404

vulnerabilities/tests/test_same_avid_different_content_id.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,20 @@ def test_latest_for_avid_tie_breaks_by_id(advisory_factory, django_assert_num_qu
6868
assert result.id == second.id
6969

7070

71+
@pytest.mark.django_db
72+
def test_latest_for_avid_returns_none_for_unknown_avid(django_assert_num_queries):
73+
"""
74+
``latest_for_avid`` should return ``None`` (not raise ``DoesNotExist``) for an
75+
avid that does not exist, since callers rely on a falsy return value to
76+
raise an ``Http404``. See GH issue: Advisory Page Returns 500 Instead of 404
77+
for Invalid Advisory IDs.
78+
"""
79+
with django_assert_num_queries(1):
80+
result = AdvisoryV2.objects.latest_for_avid("does-not-exist/ADV-404")
81+
82+
assert result is None
83+
84+
7185
@pytest.mark.django_db
7286
def test_latest_per_avid_returns_one_row_per_avid(advisory_factory, django_assert_num_queries):
7387
advisory_factory(advisory_id="A", summary="old advisory")

0 commit comments

Comments
 (0)