Skip to content
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.

Sign up for GitHub

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

Jump to bottom

lazy-wheel: be more robust with regard to Artifactory's incorrect handling of range requests with negative offsets #9082

Merged
merged 1 commit into from
Mar 1, 2024
Merged

lazy-wheel: be more robust with regard to Artifactory's incorrect handling of range requests with negative offsets #9082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions src/poetry/inspection/lazy_wheel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,12 +669,12 @@ def _extract_content_length(
return self._content_length_from_head(), None

# Some servers that do not support negative offsets,
# handle a negative offset like "-10" as "0-10".
if int(
tail.headers["Content-Length"]
) == initial_chunk_size + 1 and tail.headers["Content-Range"].startswith(
f"bytes 0-{initial_chunk_size}"
):
# handle a negative offset like "-10" as "0-10"...
# ... or behave even more strangely, see
# https://github.com/python-poetry/poetry/issues/9056#issuecomment-1973273721
if int(tail.headers["Content-Length"]) > initial_chunk_size or tail.headers.get(
"Content-Range", ""
).startswith("bytes -"):
tail = None
self._domains_without_negative_range.add(domain)
return file_length, tail
Expand Down
57 changes: 39 additions & 18 deletions tests/inspection/test_lazy_wheel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import re

from enum import IntEnum
from pathlib import Path
from typing import TYPE_CHECKING
from typing import Any
Expand Down Expand Up @@ -51,7 +52,10 @@ def __call__(
) -> None: ...


NEGATIVE_OFFSET_AS_POSITIVE = -1
class NegativeOffsetFailure(IntEnum):
# numbers must be negative to avoid conflicts with HTTP status codes
as_positive = -1 # JFrog Artifactory bug (RTDEV-38572)
one_more = -2 # JFrog Artifactory bug (one more byte than requested)


def build_head_response(
Expand All @@ -68,19 +72,26 @@ def build_partial_response(
wheel_bytes: bytes,
response_headers: dict[str, Any],
*,
negative_offset_as_positive: bool = False,
negative_offset_failure: NegativeOffsetFailure | None = None,
) -> HTTPrettyResponse:
status_code = 206
response_headers["Accept-Ranges"] = "bytes"
total_length = len(wheel_bytes)
if rng.startswith("-"):
# negative offset
offset = int(rng)
if negative_offset_as_positive:
if negative_offset_failure == NegativeOffsetFailure.as_positive:
# some servers interpret a negative offset like "-10" as "0-10"
start = 0
end = min(-offset, total_length - 1)
body = wheel_bytes[start : end + 1]
elif negative_offset_failure == NegativeOffsetFailure.one_more:
# https://github.com/python-poetry/poetry/issues/9056#issuecomment-1973273721
offset -= 1 # one more byte
start = total_length + offset # negative start of content range possible!
end = total_length - 1
body = wheel_bytes[offset:]
response_headers["Content-Length"] = -offset # just wrong...
else:
start = total_length + offset
if start < 0:
Expand Down Expand Up @@ -131,12 +142,14 @@ def handle_request(

rng = request.headers.get("Range", "=").split("=")[1]

negative_offset_as_positive = False
negative_offset_failure = None
if negative_offset_error and rng.startswith("-"):
if negative_offset_error[0] == codes.requested_range_not_satisfiable:
response_headers["Content-Range"] = f"bytes */{len(wheel_bytes)}"
if negative_offset_error[0] == NEGATIVE_OFFSET_AS_POSITIVE:
negative_offset_as_positive = True
if negative_offset_error[0] == NegativeOffsetFailure.as_positive:
negative_offset_failure = NegativeOffsetFailure.as_positive
elif negative_offset_error[0] == NegativeOffsetFailure.one_more:
negative_offset_failure = NegativeOffsetFailure.one_more
else:
return (
negative_offset_error[0],
Expand All @@ -148,7 +161,7 @@ def handle_request(
rng,
wheel_bytes,
response_headers,
negative_offset_as_positive=negative_offset_as_positive,
negative_offset_failure=negative_offset_failure,
)

status_code = 200
Expand Down Expand Up @@ -219,7 +232,8 @@ def _assertion(
(codes.requested_range_not_satisfiable, b"Requested range not satisfiable"),
(codes.internal_server_error, b"Internal server error"), # GAR
(codes.not_implemented, b"Unsupported client range"), # PyPI
(NEGATIVE_OFFSET_AS_POSITIVE, b"handle negative offset as positive"),
(NegativeOffsetFailure.as_positive, b"handle negative offset as positive"),
(NegativeOffsetFailure.one_more, b"one more byte than requested"),
],
)
def test_metadata_from_wheel_url(
Expand All @@ -236,10 +250,11 @@ def test_metadata_from_wheel_url(
# 3.-5. see negative offsets 1.-3.
expected_requests = 3
if negative_offset_error:
if negative_offset_error[0] in (
if negative_offset_error[0] in {
codes.requested_range_not_satisfiable,
NEGATIVE_OFFSET_AS_POSITIVE,
):
NegativeOffsetFailure.as_positive,
NegativeOffsetFailure.one_more,
}:
expected_requests += 1
else:
expected_requests += 2
Expand Down Expand Up @@ -299,17 +314,25 @@ def test_metadata_from_wheel_url_with_redirect(
)


@pytest.mark.parametrize("negative_offset_as_positive", [False, True])
@pytest.mark.parametrize(
("negative_offset_failure", "expected_requests"),
[
(None, 1),
(NegativeOffsetFailure.as_positive, 1),
(NegativeOffsetFailure.one_more, 2),
],
)
def test_metadata_from_wheel_url_smaller_than_initial_chunk_size(
http: type[httpretty.httpretty],
handle_request_factory: RequestCallbackFactory,
negative_offset_as_positive: bool,
negative_offset_failure: NegativeOffsetFailure | None,
expected_requests: int,
) -> None:
domain = f"tiny-wheel-{str(negative_offset_as_positive).casefold()}.com"
domain = f"tiny-wheel-{str(negative_offset_failure).casefold()}.com"
uri_regex = re.compile(f"^https://{domain}/.*$")
request_callback = handle_request_factory(
negative_offset_error=(
(NEGATIVE_OFFSET_AS_POSITIVE, b"") if negative_offset_as_positive else None
(negative_offset_failure, b"") if negative_offset_failure else None
)
)
http.register_uri(http.GET, uri_regex, body=request_callback)
Expand All @@ -324,10 +347,8 @@ def test_metadata_from_wheel_url_smaller_than_initial_chunk_size(
assert metadata["author"] == "Jason R. Coombs"
assert len(metadata["requires_dist"]) == 12

# only one request because server gives a normal response with the entire wheel
# except for when server interprets negative offset as positive
latest_requests = http.latest_requests()
assert len(latest_requests) == 1
assert len(latest_requests) == expected_requests


@pytest.mark.parametrize("accept_ranges", [None, "none"])
Expand Down
Loading