theelous3 / asks

Async requests-like httplib for python.
MIT License
508 stars 63 forks source link

erred when `Session.request(path=relative_path)` #171

Closed haolian9 closed 4 years ago

haolian9 commented 4 years ago

As current implementation, the path param of Session.request() actually should always be absolute path, otherwise, it errs.

I do not see this limit in func.__doc__, should we add some defence or just document it out?

# reproduce

from asks import Session

async def test_relpath_with_port():
    client = Session(base_location="http://example.com:80")

    resp = await client.get(path="nowhere")

    assert resp.status_code == 404

async def test_relpath_without_port():
    client = Session(base_location="http://example.com")

    resp = await client.get(path="nowhere")

    assert resp.status_code == 404
theelous3 commented 4 years ago

Leading slash for path required, which in fairness is stupid. I've just changed this on master, using urljoin. Thanks!

See: https://github.com/theelous3/asks/commit/795437e85c49256a8766a31dd4689a6a1c02d115

McSinyx commented 4 years ago

Hello there, I don't think that the new behavior is expected, e.g.

>>> urljoin('http://echo.jsontest.com/asks/test', '/1')
'http://echo.jsontest.com/1'

As a new user of this library, personally I find the previous behavior better: it's not smart, but it's consistent since it's just concatenating parts of the URL together.

theelous3 commented 4 years ago

@McSinyx whew yeah, I was in a rush there, thanks for catch.

I spent some time on this today and more completely tackled the issue: https://github.com/theelous3/asks/pull/176

McSinyx commented 4 years ago

Thanks, I didn't read the implementation but the tested behavior does what I want!