Host Whitelisting #6

kevinkjt2000 · 2017-10-30T12:01:32Z

It seems that asynctest requires at least localhost sockets when I ran my test. Would it be possible to selectively enable certain sockets and not others? For example, here I would probably get past this if I could allow localhost sockets.

============================================== FAILURES ===============================================
______________________ TestBot.test__sends_error_message_when_connection_refused ______________________
venv/lib/python3.6/site-packages/asynctest/case.py:272: in run
    self._setUp()
venv/lib/python3.6/site-packages/asynctest/case.py:217: in _setUp
    self._init_loop()
venv/lib/python3.6/site-packages/asynctest/case.py:172: in _init_loop
    loop = self.loop = asyncio.new_event_loop()
/usr/lib64/python3.6/asyncio/events.py:688: in new_event_loop
    return get_event_loop_policy().new_event_loop()
/usr/lib64/python3.6/asyncio/events.py:599: in new_event_loop
    return self._loop_factory()
/usr/lib64/python3.6/asyncio/unix_events.py:56: in __init__
    super().__init__(selector)
/usr/lib64/python3.6/asyncio/selector_events.py:67: in __init__
    self._make_self_pipe()
/usr/lib64/python3.6/asyncio/selector_events.py:129: in _make_self_pipe
    self._ssock, self._csock = self._socketpair()
/usr/lib64/python3.6/asyncio/unix_events.py:60: in _socketpair
    return socket.socketpair()
/usr/lib64/python3.6/socket.py:489: in socketpair
    a = socket(family, type, proto, a.detach())
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

args = (<AddressFamily.AF_UNIX: 1>, <SocketKind.SOCK_STREAM: 1>, 0, 11), kwargs = {}

    def guarded(*args, **kwargs):
>       raise SocketBlockedError()
E       pytest_socket.SocketBlockedError: A test tried to use socket.socket.

venv/lib/python3.6/site-packages/pytest_socket.py:56: SocketBlockedError
================================= 1 failed, 4 passed in 0.41 seconds ==================================
Exception ignored in: <bound method BaseEventLoop.__del__ of <_UnixSelectorEventLoop running=False closed=False debug=False>>
Traceback (most recent call last):
  File "/usr/lib64/python3.6/asyncio/base_events.py", line 512, in __del__
    self.close()
  File "/usr/lib64/python3.6/asyncio/unix_events.py", line 63, in close
    super().close()
  File "/usr/lib64/python3.6/asyncio/selector_events.py", line 110, in close
    self._close_self_pipe()
  File "/usr/lib64/python3.6/asyncio/selector_events.py", line 120, in _close_self_pipe
    self._remove_reader(self._ssock.fileno())
AttributeError: '_UnixSelectorEventLoop' object has no attribute '_ssock'

To reproduce:
pytest.ini

[pytest]
addopts = --disable-socket

test.py

import asynctest

class TestClass(asynctest.TestCase):
    async def test_something(self):
        pass

commands to enter:

pip install pytest pytest-socket asynctest
pytest test.py

The text was updated successfully, but these errors were encountered:

miketheman · 2017-10-31T12:16:10Z

Hi @kevinkjt2000 !

Thanks for the awesome report.
(minor note, test case should pass async def test_something(self): or I get a TypeError)

Right now, the granularity of the logic is very coarse - it's set at the test case level - so it's not set to distinguish a difference between types of sockets within a test case. So we can selectively enable socket use for the entire test case using fixtures, but then run the risk of the items within the async test case making network calls.

I also was curious about your use of asynctest vs pytest-asyncio - I was able to simplify the case to this code, allowing the async test to pass:

import pytest
import pytest_socket


@pytest.mark.asyncio
async def test_something(socket_enabled):
    pass

Again, this doesn't have the granularity of detecting what happens during the test case, as socket is now enabled.

I was thinking about how that might be implemented, and haven't come up with a decent design - as it would require evaluating within the test case any time socket is called to determine the AddressFamily in use.

kevinkjt2000 · 2017-10-31T16:46:36Z

Well I could enable socket and add self to get the asynctest to pass too xD (I edited my original comment to include self)

As for how to design this, I would suggest some kind of lookup list in the fake disabled socket? If it is allowed, let it use real socket; otherwise, send it to fake socket? I am ignorant as to how blocking all sockets is implemented currently.

sadams · 2018-06-18T11:47:32Z

I'll have a go if no one else has started...

Before i go too far, @miketheman do you have any views on the following:

instead of overriding socket.socket we can intercept socket.socket.connect (to give us the host)
optionally provide a list of allowed hosts like this: disable_socket(['127.0.0.1']) or for cli --disable-socket 127.0.0.1,127.0.1.1 (alternatively we could create a separate method called disable_socket_connect(...) if that is better)
because of the above, i think ip addresses rather than hostnames will have to be used (i think connect gets the resolved IP address rather than the hostname). However, I can add something to do the docs showing how to use getaddrinfo to help with that.

Cheers

miketheman · 2018-06-19T11:27:03Z

Hey @sadams! Awesome that you're going for it.

For the questions asked, some thoughts.

If overriding the subset of the socket, are there other ways one might use it that don't directly use connect that would circumvent the disable? I guess a look at uses of socket across GitHub might be an indication, but not a perfect sample, since people do some crazy things behind closed repos.
The config interface is one I also am struggling with to get a clean approach, since the main desire is to disable sockets, and then another option to selectively enable for some hosts, unless you're thinking that the disable action would only disable the listed hosts?
What happens currently if a socket.gethostbyname is called? If DNS resolution works when disabling socket, then that could be a bug, since DNS resolution is a network call, and the point of this plugin was to disable them. So I'm not sure how the hostname/ip address should work just yet.

sadams · 2018-06-19T12:16:27Z

Hi @miketheman ,
Good point about socket being used in DNS resolution, but I think it's not using python-level sockets at all:

import socket

def foo():
    print('overridden')

socket.socket = foo

print(socket.gethostbyname('google.com'))
# 172.217.23.46

So that is 'technically' a bug in the original, but I think it's probably pretty rare that people are using this to avoid DNS side effects.

sadams · 2018-06-19T12:17:53Z

On the other points, i ended up doing the code, so i will submit a PR and we can discuss there (basically i added an optional argument to disable_sockets that was a whitelist of hosts).

miketheman · 2018-06-26T13:24:43Z

I may have closed this prematurely - the solution in #9 appears to work for connections with designated IP addresses, but the repro case here for unix sockets remains unsolved.

kamikaze · 2018-12-06T02:33:36Z

got same problem... allowing AF_UNIX sockets while disabling AF_INET could be nice to have

* Remove unnecessary comments. * Remove `pytest-socket` as it does not work with aiohttp. See miketheman/pytest-socket#6 for reference.

* Remove unnecessary comments. * Remove `pytest-socket` as it does not work with aiohttp. See miketheman/pytest-socket#6 for reference. * Enable `mergify` for this repository.

benhowes · 2020-01-24T15:23:24Z

I have taken a look at this because I needed to be able to whitelist hostnames (i.e. localhost rather than127.0.0.1, which I think is one of the cases this was asking about. In my case, I needed to be able to whitelist redis which is in the same docker-compose network.

I've created a fork with my changes which boils down to the addition of this function: https://github.com/benhowes/pytest-socket/blob/master/pytest_socket.py#L129-L157

It's not in a great shape for a PR at the moment since I opted to ditch python 2.7 and 3.3 at the same time because I wanted to use the ipaddress module which was added in 3.4.

Before I do any work on making it more PR ready, is there an appetite for this @miketheman ?

Alexander3 · 2020-05-20T09:38:43Z

I have similar problem in tests for Django project (it appeared after upgrade to django 3.* because it's using async underneath)

jamesbeith · 2020-05-25T06:27:25Z

Also getting this problem when running pytest with --disable-socket after having upgraded to Django 3.0.6.

Tests (that access the database, possibly related) fail with:

Exception ignored in: <function BaseEventLoop.__del__ at 0x7ffb4de03cb0>
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/asyncio/base_events.py", line 620, in __del__
    self.close()
  File "/usr/local/lib/python3.7/asyncio/unix_events.py", line 55, in close
    super().close()
  File "/usr/local/lib/python3.7/asyncio/selector_events.py", line 86, in close
    self._close_self_pipe()
  File "/usr/local/lib/python3.7/asyncio/selector_events.py", line 93, in _close_self_pipe
    self._remove_reader(self._ssock.fileno())
AttributeError: '_UnixSelectorEventLoop' object has no attribute '_ssock'

and then start failing with:

...
/venv/lib/python3.7/site-packages/django/db/models/manager.py:82: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
/venv/lib/python3.7/site-packages/django/db/models/query.py:559: in get_or_create
    return self.get(**kwargs), False
/venv/lib/python3.7/site-packages/django/db/models/query.py:411: in get
    num = len(clone)
/venv/lib/python3.7/site-packages/django/db/models/query.py:258: in __len__
    self._fetch_all()
/venv/lib/python3.7/site-packages/django/db/models/query.py:1261: in _fetch_all
    self._result_cache = list(self._iterable_class(self))
/venv/lib/python3.7/site-packages/django/db/models/query.py:57: in __iter__
    results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
/venv/lib/python3.7/site-packages/django/db/models/sql/compiler.py:1149: in execute_sql
    cursor = self.connection.cursor()
/venv/lib/python3.7/site-packages/django/utils/asyncio.py:19: in inner
    event_loop = asyncio.get_event_loop()
/usr/local/lib/python3.7/asyncio/events.py:640: in get_event_loop
    self.set_event_loop(self.new_event_loop())
/usr/local/lib/python3.7/asyncio/events.py:660: in new_event_loop
    return self._loop_factory()
/usr/local/lib/python3.7/asyncio/unix_events.py:51: in __init__
    super().__init__(selector)
/usr/local/lib/python3.7/asyncio/selector_events.py:55: in __init__
    self._make_self_pipe()
/usr/local/lib/python3.7/asyncio/selector_events.py:102: in _make_self_pipe
    self._ssock, self._csock = socket.socketpair()
/usr/local/lib/python3.7/socket.py:491: in socketpair
    a, b = _socket.socketpair(family, type, proto)
OSError: [Errno 24] Too many open files

When running without --disable-socket all tests pass.

Alexander3 · 2020-06-07T17:41:28Z

I'm also getting
AttributeError: '_UnixSelectorEventLoop' object has no attribute '_ssock'
and even
pytest_socket.SocketBlockedError: A test tried to use socket.socket.
in tests involving aiohttp when running with config like this:

;setup.cfg file
[tool:pytest]
addopts = --ff --tb=short --allow-hosts=127.0.0.1,::1 -q

That's strange because as I understand --allow-hosts should never cause SocketBlockedError.

In tests without async stuff if I do something like

    import requests
    requests.get("http://google.com")

It gives: pytest_socket.SocketConnectBlockedError: A test tried to use socket.socket.connect() with host "216.58.215.78" (allowed: "127.0.0.1,::1"). as expected.

[pytest-socket==0.3.5]

joetsoi · 2021-02-25T13:40:59Z

@jamesbeith I've run this on our CI with the pr above and it seems to be passing, will need to check that all the failures are real http requests, but looks promising so far

👋 @alex-verve , long time no see, do you mind running it on your codebase and giving it a go if it's still useful that is!

miketheman · 2021-03-28T21:24:32Z

Thanks to @joetsoi 's efforts, the patch is merged and I'll be cutting a release in the next few days. Closing this issue!

While the basics of asyncio ought to be covered by enabling unix sockets in #54, I thought it might be nice to add some explicit asyncio tests to ensure that we don't hit any framework-specific regressions. I had written these locally when testing the changes anyhow. Refs: #6 Refs: #47 Signed-off-by: Mike Fiedler <miketheman@gmail.com>

sondrelg · 2021-05-27T12:35:19Z

I have taken a look at this because I needed to be able to whitelist hostnames (i.e. localhost rather than127.0.0.1, which I think is one of the cases this was asking about. In my case, I needed to be able to whitelist redis which is in the same docker-compose network.

I've created a fork with my changes which boils down to the addition of this function: https://github.com/benhowes/pytest-socket/blob/master/pytest_socket.py#L129-L157

It's not in a great shape for a PR at the moment since I opted to ditch python 2.7 and 3.3 at the same time because I wanted to use the ipaddress module which was added in 3.4.

Before I do any work on making it more PR ready, is there an appetite for this @miketheman ?

I've just run into the same issue trying to set up pytest with redis in a Github workflow. Essentially when running tests in a container the redis service is accessible via the redis hostname (see docs) - I guess it's the exact same situation you would run into in a docker-compose setup if your application was running as a container.

Eventually I ended up resolving the issue with this:

      - name: Get Redis IP
        id: redis
        run: |
          redis_ip=$(echo "import socket; print(socket.gethostbyname('redis'))" | python)
          echo ::set-output name=ip::"$redis_ip"

      - run: pytest ... --allow-hosts=127.0.0.1,${{ steps.redis.outputs.ip }}
        env:
          REDIS_HOST: redis
          REDIS_PORT: ${{ job.services.redis.ports[6379] }}

But ideally, I would want to be able to just do this

      - run: pytest ... --allow-hosts=127.0.0.1,redis
        env:
          REDIS_HOST: redis
          REDIS_PORT: ${{ job.services.redis.ports[6379] }}

Are you still planning on submitting a PR for this feature @benhowes, or has an equivalent fix already been added?

benhowes · 2021-05-28T16:35:47Z

@sondrelg I'm no longer working on a project which uses my fork and I've really lost track of where this library. Sorry!

miketheman added the help wanted Extra attention is needed label Nov 23, 2017

miketheman mentioned this issue Jun 2, 2018

Issues with asyncio #7

Closed

This was referenced Jun 19, 2018

add ability to whitelist hosts when disabling socket connect #8

Closed

create second argument for restricting connections to specific hosts #9

Merged

miketheman closed this as completed in 07e1c89 Jun 26, 2018

miketheman reopened this Jun 26, 2018

rgreinho added a commit to rgreinho/yelper that referenced this issue Jan 16, 2019

Clean up

d6e8784

* Remove unnecessary comments. * Remove `pytest-socket` as it does not work with aiohttp. See miketheman/pytest-socket#6 for reference.

rgreinho mentioned this issue Jan 16, 2019

Clean up rgreinho/yelper#7

Merged

rgreinho added a commit to rgreinho/yelper that referenced this issue Jan 16, 2019

Clean up

b7acfec

* Remove unnecessary comments. * Remove `pytest-socket` as it does not work with aiohttp. See miketheman/pytest-socket#6 for reference.

rgreinho added a commit to rgreinho/yelper that referenced this issue Jan 16, 2019

Clean up

0693cfe

* Remove unnecessary comments. * Remove `pytest-socket` as it does not work with aiohttp. See miketheman/pytest-socket#6 for reference. * Enable `mergify` for this repository.

rgreinho added a commit to rgreinho/yelper that referenced this issue Jan 16, 2019

Clean up (#7)

7a7c4ec

* Remove unnecessary comments. * Remove `pytest-socket` as it does not work with aiohttp. See miketheman/pytest-socket#6 for reference. * Enable `mergify` for this repository.

simonfagerholm mentioned this issue May 14, 2020

v0.12.0 is incompatible with pytest-socket + mocked aiohttp request (aioresponses) pytest-dev/pytest-asyncio#160

Closed

miketheman mentioned this issue Oct 11, 2020

Does not work with FastAPI/starlette's TestClient #47

Closed

joetsoi mentioned this issue Feb 25, 2021

Don't block unix sockets #54

Merged

miketheman closed this as completed Mar 28, 2021

miketheman mentioned this issue Mar 30, 2021

test: add testing for some async frameworks #63

Merged

miketheman mentioned this issue Oct 15, 2022

Question about asynctest and Python 3.8+ compatibility #161

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Host Whitelisting #6

Host Whitelisting #6

kevinkjt2000 commented Oct 30, 2017 •

edited

Loading

miketheman commented Oct 31, 2017

kevinkjt2000 commented Oct 31, 2017 •

edited

Loading

sadams commented Jun 18, 2018 •

edited

Loading

miketheman commented Jun 19, 2018

sadams commented Jun 19, 2018 •

edited

Loading

sadams commented Jun 19, 2018

miketheman commented Jun 26, 2018

kamikaze commented Dec 6, 2018

benhowes commented Jan 24, 2020

Alexander3 commented May 20, 2020

jamesbeith commented May 25, 2020 •

edited

Loading

Alexander3 commented Jun 7, 2020

joetsoi commented Feb 25, 2021 •

edited

Loading

miketheman commented Mar 28, 2021

sondrelg commented May 27, 2021

benhowes commented May 28, 2021

Host Whitelisting #6

Host Whitelisting #6

Comments

kevinkjt2000 commented Oct 30, 2017 • edited Loading

miketheman commented Oct 31, 2017

kevinkjt2000 commented Oct 31, 2017 • edited Loading

sadams commented Jun 18, 2018 • edited Loading

miketheman commented Jun 19, 2018

sadams commented Jun 19, 2018 • edited Loading

sadams commented Jun 19, 2018

miketheman commented Jun 26, 2018

kamikaze commented Dec 6, 2018

benhowes commented Jan 24, 2020

Alexander3 commented May 20, 2020

jamesbeith commented May 25, 2020 • edited Loading

Alexander3 commented Jun 7, 2020

joetsoi commented Feb 25, 2021 • edited Loading

miketheman commented Mar 28, 2021

sondrelg commented May 27, 2021

benhowes commented May 28, 2021

kevinkjt2000 commented Oct 30, 2017 •

edited

Loading

kevinkjt2000 commented Oct 31, 2017 •

edited

Loading

sadams commented Jun 18, 2018 •

edited

Loading

sadams commented Jun 19, 2018 •

edited

Loading

jamesbeith commented May 25, 2020 •

edited

Loading

joetsoi commented Feb 25, 2021 •

edited

Loading