fix: refget endpoint reported an incorrect Content-Length, leading to RuntimeErrors

pyproject.toml

@@ -111,8 +111,8 @@ testpaths = "tests/"
 pythonpath = "."
 norecursedirs = "tests/helpers/"
 markers = """
-    integration: tests that exercise multi-component flows and/or database interactions.
-    unit: fast unit tests with isolated dependencies.
+    integration: tests that exercise a multi-component flow end-to-end, with no internal mocking.
+    unit: fast, isolated tests. Real un-mocked collaborators are fine if they're local, fast, and deterministic.
     network: tests which interact with external services over a network connection.
     slow: tests which are slow running.
 """

src/mavedb/routers/refget.py

@@ -158,7 +158,12 @@ def get_sequence(
             logger.error(msg="Invalid range header format", extra=logging_context())
             raise HTTPException(status_code=400, detail="Invalid range header format")
 
+        # Rejects "bytes=5-4"-style headers (RFC 9110 S14.1.1): after conversion to a half-open
+        # bound, start == end here would otherwise silently pass as a valid empty range.
         start, end = int(m.group(1)), int(m.group(2)) + 1
+        if start >= end:
+            logger.error(msg="Invalid range header format", extra=logging_context())
+            raise HTTPException(status_code=400, detail="Invalid range header format")
 
     save_to_logging_context({"requested_refget_start": start, "requested_refget_end": end})
     if start is not None and end is not None:
@@ -183,37 +188,59 @@ def get_sequence(
         )
 
     seq_id = seq_ids[0]
-    seqinfo = sr.sequences.fetch_seqinfo(seq_id)
+    seq_len = sr.sequences.fetch_seqinfo(seq_id)["len"]
+
+    # Resolve to concrete half-open bounds up front so validation, Content-Length, and the streamed
+    # body all agree, even when only one of start/end is supplied.
+    seq_start = start if start is not None else 0
+    seq_end = end if end is not None else seq_len
 
+    # Refget spec: only a start "larger than" the sequence length is invalid (strict inequality).
     if start is not None and end is not None:
-        if start >= seqinfo["len"]:
+        if seq_start > seq_len:
             raise HTTPException(
                 status_code=416,
                 detail="Invalid coordinates: start > sequence length",
-                headers={"Content-Range": f"bytes */{seqinfo['len']}"},
+                headers={"Content-Range": f"bytes */{seq_len}"},
             )
-        if end > seqinfo["len"]:
+        if seq_end > seq_len:
             raise HTTPException(
                 status_code=416,
                 detail="Invalid coordinates: end > sequence length",
-                headers={"Content-Range": f"bytes */{seqinfo['len']}"},
+                headers={"Content-Range": f"bytes */{seq_len}"},
             )
-        if not (0 <= start <= end <= seqinfo["len"]):
+        if not (0 <= seq_start <= seq_end <= seq_len):
             raise HTTPException(
                 status_code=416,
                 detail="Invalid coordinates: must obey 0 <= start <= end <= sequence_length",
-                headers={"Content-Range": f"bytes */{seqinfo['len']}"},
+                headers={"Content-Range": f"bytes */{seq_len}"},
             )
+    # The refget spec only requires rejecting a start beyond the sequence length; there's no
+    # equivalent rule for a lone end (see below).
+    elif start is not None:
+        if not 0 <= seq_start <= seq_len:
+            raise HTTPException(
+                status_code=400,
+                detail=f"Invalid coordinates: start must satisfy 0 <= start <= {seq_len}",
+            )
+    # Clamp the end coordinate to the valid range [0, seq_len].
+    elif end is not None:
+        seq_end = max(0, min(seq_end, seq_len))
 
-    headers = {"Content-Length": str(seqinfo["len"])}
-    if start is not None and end is not None and range_header:
+    # Content-Length must match bytes actually streamed. Overstating it aborts the response mid-stream
+    # once the ASGI server has already committed the status line.
+    headers = {"Content-Length": str(seq_end - seq_start)}
+    if range_header:
         status = 206
-        headers["Content-Range"] = f"bytes {start}-{end - 1}/{seqinfo['len']}"
+        headers["Content-Range"] = f"bytes {seq_start}-{seq_end - 1}/{seq_len}"
         headers["Accept-Ranges"] = "bytes"
     else:
         status = 200
         headers["Accept-Ranges"] = "none"
 
     return StreamingResponse(
-        sequence_generator(sr, seq_ids[0], start, end), media_type="text/plain", status_code=status, headers=headers
+        sequence_generator(sr, seq_id, seq_start, seq_end),
+        media_type="text/plain",
+        status_code=status,
+        headers=headers,
     )

src/mavedb/routers/seqrepo.py

@@ -71,7 +71,31 @@ def get_sequence(
             status_code=400, detail=f"Multiple sequences exist for alias '{alias}'. Use an explicit namespace."
         )
 
-    return StreamingResponse(sequence_generator(sr, seq_ids[0], start, end), media_type="text/plain")
+    seq_id = seq_ids[0]
+    seq_len = sr.sequences.fetch_seqinfo(seq_id)["len"]
+
+    # Resolve to concrete half-open bounds so the check and the streamed body agree, even when only
+    # one of start/end is supplied.
+    seq_start = start if start is not None else 0
+    seq_end = end if end is not None else seq_len
+
+    # Mirrors refget.py's two-sided rule: start == seq_len (an empty tail slice) is legal.
+    if start is not None and end is not None:
+        if not 0 <= seq_start <= seq_end <= seq_len:
+            logger.error(msg="Invalid coordinates: range lies outside the sequence.", extra=logging_context())
+            raise HTTPException(
+                status_code=422, detail=f"Invalid coordinates: must obey 0 <= start <= end <= {seq_len}"
+            )
+    # Mirrors refget.py: a start beyond the sequence length is rejected.
+    elif start is not None:
+        if not 0 <= seq_start <= seq_len:
+            logger.error(msg="Invalid coordinates: start lies outside the sequence.", extra=logging_context())
+            raise HTTPException(status_code=422, detail=f"Invalid coordinates: must obey 0 <= start <= {seq_len}")
+    # Mirrors refget.py: an over-long end is clamped rather than rejected.
+    elif end is not None:
+        seq_end = max(0, min(seq_end, seq_len))
+
+    return StreamingResponse(sequence_generator(sr, seq_id, seq_start, seq_end), media_type="text/plain")
 
 
 @router.get("/metadata/{alias}", response_model=SeqRepoMetadata, summary="Get sequence metadata by alias")

tests/routers/test_refget.py

@@ -3,6 +3,8 @@
 
 import pytest
 
+pytestmark = pytest.mark.unit
+
 arq = pytest.importorskip("arq")
 cdot = pytest.importorskip("cdot")
 fastapi = pytest.importorskip("fastapi")
@@ -12,6 +14,24 @@
 from tests.helpers.constants import TEST_SEQREPO_INITIAL_STATE, VALID_ENSEMBL_IDENTIFIER
 
 
+@pytest.mark.parametrize(
+    "env_value,expected_data_version",
+    [(None, "unknown"), ("/some/path/seqrepo/20240101", "20240101")],
+)
+def test_service_info(client, monkeypatch, env_value, expected_data_version):
+    if env_value is None:
+        monkeypatch.delenv("HGVS_SEQREPO_DIR", raising=False)
+    else:
+        monkeypatch.setenv("HGVS_SEQREPO_DIR", env_value)
+
+    resp = client.get("/api/v1/refget/sequence/service-info")
+    assert resp.status_code == 200
+    data = resp.json()
+    assert data["name"] == "MaveDB API"
+    assert data["seqrepo_data_version"] == expected_data_version
+    assert data["refget"]["identifier_types"] == ["refseq", "ensembl"]
+
+
 @pytest.mark.parametrize("entry", TEST_SEQREPO_INITIAL_STATE)
 def test_get_metadata_success(client, entry):
     alias = list(entry.keys())[0]
@@ -76,6 +96,86 @@ def test_get_sequence_with_range_query(client, entry):
     assert resp.text == metadata["seq"][start:end]
 
 
+@pytest.mark.parametrize("entry", TEST_SEQREPO_INITIAL_STATE)
+def test_get_sequence_only_start(client, entry):
+    alias = list(entry.keys())[0]
+    metadata = list(entry.values())[0]
+    start = 1
+    resp = client.get(f"/api/v1/refget/sequence/{alias}", params={"start": start})
+    assert resp.status_code == 200
+    assert resp.text == metadata["seq"][start:]
+
+
+@pytest.mark.parametrize("entry", TEST_SEQREPO_INITIAL_STATE)
+def test_get_sequence_only_end(client, entry):
+    alias = list(entry.keys())[0]
+    metadata = list(entry.values())[0]
+    end = 3
+    resp = client.get(f"/api/v1/refget/sequence/{alias}", params={"end": end})
+    assert resp.status_code == 200
+    assert resp.text == metadata["seq"][:end]
+
+
+def test_get_sequence_range_header_and_query_params_conflict(client):
+    resp = client.get(
+        f"/api/v1/refget/sequence/{VALID_ENSEMBL_IDENTIFIER}",
+        params={"start": 1},
+        headers={"Range": "bytes=1-2"},
+    )
+    assert resp.status_code == 400
+    assert "Cannot use both start/end query parameters and Range header" in resp.text
+
+
+def test_get_sequence_invalid_query_range_only_start_negative(client):
+    resp = client.get(f"/api/v1/refget/sequence/{VALID_ENSEMBL_IDENTIFIER}", params={"start": -1})
+    assert resp.status_code == 400
+    assert "Invalid coordinates" in resp.text
+
+
+def test_get_sequence_invalid_query_range_only_start_too_large(client):
+    resp = client.get(f"/api/v1/refget/sequence/{VALID_ENSEMBL_IDENTIFIER}", params={"start": 7})
+    assert resp.status_code == 400
+    assert "Invalid coordinates" in resp.text
+
+
+def test_get_sequence_only_end_too_large_clamps(client):
+    # Unlike start, an over-long lone end is clamped rather than rejected.
+    resp = client.get(f"/api/v1/refget/sequence/{VALID_ENSEMBL_IDENTIFIER}", params={"end": 10})
+    assert resp.status_code == 200
+    assert resp.text == "GGGG"
+
+
+@pytest.mark.parametrize(
+    "params,headers",
+    [
+        ({}, {}),
+        ({"start": 1, "end": 3}, {}),
+        ({"start": 1}, {}),
+        ({"end": 3}, {}),
+        ({}, {"Range": "bytes=1-3"}),
+    ],
+)
+def test_get_sequence_content_length_matches_body(client, params, headers):
+    resp = client.get(
+        f"/api/v1/refget/sequence/{VALID_ENSEMBL_IDENTIFIER}",
+        params=params,
+        # Accept-Encoding: identity is required — GZipMiddleware strips Content-Length from compressed responses.
+        headers={**headers, "Accept-Encoding": "identity"},
+    )
+    assert resp.status_code in (200, 206)
+    assert resp.headers["Content-Length"] == str(len(resp.content))
+
+
+def test_get_sequence_range_header_reports_requested_span(client):
+    resp = client.get(
+        f"/api/v1/refget/sequence/{VALID_ENSEMBL_IDENTIFIER}",
+        headers={"Range": "bytes=1-2", "Accept-Encoding": "identity"},
+    )
+    assert resp.status_code == 206
+    assert resp.headers["Content-Range"] == "bytes 1-2/4"
+    assert resp.headers["Content-Length"] == "2"
+
+
 def test_get_sequence_not_found(client):
     resp = client.get("/api/v1/refget/sequence/notfound")
     assert resp.status_code == 404
@@ -136,3 +236,16 @@ def test_get_sequence_range_header_invalid(client):
     resp = client.get(f"/api/v1/refget/sequence/{VALID_ENSEMBL_IDENTIFIER}", headers=headers)
     assert resp.status_code == 400
     assert "Invalid range header format" in resp.text
+
+
+def test_get_sequence_range_header_invalid_inverted_by_one(client):
+    resp = client.get(f"/api/v1/refget/sequence/{VALID_ENSEMBL_IDENTIFIER}", headers={"Range": "bytes=5-4"})
+    assert resp.status_code == 400
+    assert "Invalid range header format" in resp.text
+
+
+def test_get_sequence_range_header_single_byte(client):
+    # Boundary case adjacent to the inverted range above: first-byte-pos == last-byte-pos is valid.
+    resp = client.get(f"/api/v1/refget/sequence/{VALID_ENSEMBL_IDENTIFIER}", headers={"Range": "bytes=1-1"})
+    assert resp.status_code == 206
+    assert resp.text == "G"

tests/routers/test_seqrepo.py

@@ -3,6 +3,8 @@
 
 import pytest
 
+pytestmark = pytest.mark.unit
+
 arq = pytest.importorskip("arq")
 cdot = pytest.importorskip("cdot")
 fastapi = pytest.importorskip("fastapi")
@@ -46,12 +48,55 @@ def test_get_sequence_multiple_ids(client):
         assert "Multiple sequences exist" in resp.text
 
 
+@pytest.mark.parametrize("entry", TEST_SEQREPO_INITIAL_STATE)
+def test_get_sequence_only_start(client, entry):
+    alias = list(entry.keys())[0]
+    metadata = list(entry.values())[0]
+    start = 1
+    resp = client.get(f"/api/v1/seqrepo/sequence/{alias}?start={start}")
+    assert resp.status_code == 200
+    assert resp.text == metadata["seq"][start:]
+
+
+@pytest.mark.parametrize("entry", TEST_SEQREPO_INITIAL_STATE)
+def test_get_sequence_only_end(client, entry):
+    alias = list(entry.keys())[0]
+    metadata = list(entry.values())[0]
+    end = 3
+    resp = client.get(f"/api/v1/seqrepo/sequence/{alias}?end={end}")
+    assert resp.status_code == 200
+    assert resp.text == metadata["seq"][:end]
+
+
 def test_get_sequence_invalid_coords(client):
     resp = client.get(f"/api/v1/seqrepo/sequence/{VALID_ENSEMBL_IDENTIFIER}?start=10&end=5")
     assert resp.status_code == 422
     assert "Invalid coordinates" in resp.text
 
 
+# Coordinates outside the sequence used to stream a truncated body under a 200 instead of being rejected.
+@pytest.mark.parametrize(
+    "query",
+    [
+        "start=10&end=12",
+        "start=1&end=12",
+        "start=10",
+        "start=-1&end=2",
+    ],
+)
+def test_get_sequence_coords_outside_sequence(client, query):
+    resp = client.get(f"/api/v1/seqrepo/sequence/{VALID_ENSEMBL_IDENTIFIER}?{query}")
+    assert resp.status_code == 422
+    assert "Invalid coordinates" in resp.text
+
+
+def test_get_sequence_only_end_too_large_clamps(client):
+    # Mirrors refget.py: an over-long lone end is clamped rather than rejected.
+    resp = client.get(f"/api/v1/seqrepo/sequence/{VALID_ENSEMBL_IDENTIFIER}?end=12")
+    assert resp.status_code == 200
+    assert resp.text == "GGGG"
+
+
 @pytest.mark.parametrize("entry", TEST_SEQREPO_INITIAL_STATE)
 def test_get_metadata_success(client, entry):
     alias = list(entry.keys())[0]