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.

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

RemotePeer configuration has suspicious handling of port #419

Closed
DMRobertson opened this issue Oct 11, 2021 · 0 comments · Fixed by #420
Closed

RemotePeer configuration has suspicious handling of port #419

DMRobertson opened this issue Oct 11, 2021 · 0 comments · Fixed by #420
Assignees

Comments

@DMRobertson
Copy link
Contributor

DMRobertson commented Oct 11, 2021

The peers table is responsible for generating RemotePeer objetcs. RemotePeer is responsible for (amongst other things) working out which replication url to use.

  • We'll use the port defined in the configuration url (possibly 443 implicitly)
  • If there's no replication url configured, we'll use the port provided in the call to RemotePeer.__init__
    • If port is Falsey then use 1001; else use

After this, the only uses of port and self.port are for logging. We always log with %d, so expect this to be an integer.

There is a path here where you can do a bad thing.

  1. Don't configure any base replication urls
  2. Add an entry to peers with a NULL port
  3. Every call to RemotePeer will try to format None as an integer, which will raise a TypeError.
>>> "%d" % None
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: %d format: a number is required, not NoneType

Equally, if we did specify a port in the DB and did provide a replication URL, the db's port would be ignored, but the logging would continue to print it anyway!

Fix this by logging the base replication url so we don't have any need for the port.

@DMRobertson DMRobertson self-assigned this Oct 11, 2021
DMRobertson pushed a commit that referenced this issue Oct 11, 2021
It's niche, but if no replication base url was configured and a peer was
configured with a NULL port in the db, then we would try to format None
as an integer and get a TypeError for our troubles.

Spotted by work in progress elsewhere to make Sydent pass mypy --strict.

Fixes #419.
DMRobertson pushed a commit that referenced this issue Oct 12, 2021
It's niche, but if no replication base url was configured and a peer was
configured with a NULL port in the db, then we would try to format None
as an integer and get a TypeError for our troubles.

Spotted by work in progress elsewhere to make Sydent pass mypy --strict.

Fixes #419.

Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant