Skip to content

--listen parameter lets one specify nonsensical behaviour #153

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.

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

Closed
Ichimonji10 opened this issue Mar 5, 2020 · 5 comments
Closed

--listen parameter lets one specify nonsensical behaviour #153

Ichimonji10 opened this issue Mar 5, 2020 · 5 comments
Labels
bug Something isn't working needs_test This item needs to be tested

Comments

@Ichimonji10
Copy link
Contributor

Ichimonji10 commented Mar 5, 2020

I can start receptor like so:

poetry run receptor --data-dir="$(mktemp --directory)" node --listen=receptor://127.0.0.1:8888

This works fine. But let's say I start receptor like so:

poetry run receptor --data-dir="$(mktemp --directory)" node --listen=receptor://127.0.0.1:8888/foo

In this case, receptor will (to the best of my knowledge) silently discard the path /foo. This seems problematic to me. The user might expect one thing ("receptor will nest its listen interface under the /foo path"), but receptor will do something else. This could be a source of user confusion.

If receptor is going to discard the path, then why does it allow a path to be specified in the first place? Perhaps receptor should reject --listen arguments with information that'd be dropped anyway. For example, the following URL could be rejected:

>>> from urllib.parse import urlparse
>>> urlparse('receptor://127.0.0.1/0:8888')
ParseResult(scheme='receptor', netloc='127.0.0.1', path='/0:8888', params='', query='', fragment='')

The logic could be something like:

if res.path or res.params or res.query or res.fragment:
    raise InvalidListenArgumentError(...)
@Ichimonji10
Copy link
Contributor Author

Ichimonji10 commented Mar 5, 2020

Alternately, receptor could provide parameters like --listen-scheme, --listen-address and --listen-port. That'd be a much more verbose option, though, and it doesn't strike me as being as elegant.

@ghjm
Copy link
Contributor

ghjm commented Mar 5, 2020

I can get behind the idea of making it an error to provide path information in the URL that won't be used. That would deal with the receptor://0.0.0.0/0:8888 case.

Does path information possibly mean something in the case of a websocket listener?

@Ichimonji10
Copy link
Contributor Author

Does path information possibly mean something in the case of a websocket listener?

Good question. I've no idea. 🤷‍♀️ @matburt ?

@Ichimonji10
Copy link
Contributor Author

Also worth thinking about: can we tell receptor to bind to an interface, and let it discover that interface' address at runtime? That functionality is both common and useful, and it might affect how this issue is addressed.

@ghjm ghjm added the bug Something isn't working label Mar 6, 2020
@ghjm ghjm added the needs_test This item needs to be tested label Mar 9, 2020
@Ichimonji10
Copy link
Contributor Author

Fixed by #157. Automated tests added in #139.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs_test This item needs to be tested
Projects
None yet
Development

No branches or pull requests

2 participants