/synapse

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

add a url_preview_ip_range_whitelist config param #760

Merged
merged 3 commits into from May 16, 2016

Conversation

Reviewers
No reviews
Assignees

@NegativeMjark NegativeMjark

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@ara4n @NegativeMjark
@ara4n
Owner

ara4n commented May 1, 2016

...so we can whitelist the matrix.org IP space
@ara4n
add a url_preview_ip_range_whitelist config param so we can whitelist… 
… the matrix.org IP space

Some checks were not successful

2 errored, 2 failing, and 4 successful checks
@matrixbot
Sytest Postgres (Commit) — Build #563 origin/matthew/preview_url_ip_whitelist failed in 4 min 41 sec
Details
@matrixbot
Sytest SQLite (Commit) — Build #570 origin/matthew/preview_url_ip_whitelist failed in 3 min 17 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Flake8 + Packaging (Commit) — Build #579 origin/matthew/preview_url_ip_whitelist succeeded in 30 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #623 origin/matthew/preview_url_ip_whitelist succeeded in 1 min 13 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
792def4

@ara4n ara4n assigned NegativeMjark May 1, 2016

@NegativeMjark NegativeMjark commented on an outdated diff May 3, 2016

View full outdated diff
synapse/http/client.py
@@ -380,13 +380,15 @@ def post_urlencoded_get_raw(self, url, args={}):
class SpiderEndpointFactory(object):
def __init__(self, hs):
self.blacklist = hs.config.url_preview_ip_range_blacklist
+ if hasattr(hs.config, "url_preview_ip_range_whitelist"):
+ self.whitelist = hs.config.url_preview_ip_range_whitelist
@NegativeMjark

NegativeMjark May 3, 2016

Contributor

You will want to set self.whitelist even if there isn't a url_preview_ip_range_whitelist in the config.

Perhaps self.whitelist = getattr(hs.config, "url_preview_ip_range_whitelist", None)?
@ara4n
Owner

ara4n commented May 4, 2016

@NegativeMjark thanks - anything else?

@NegativeMjark NegativeMjark commented on an outdated diff May 4, 2016

View full outdated diff
synapse/config/repository.py
@@ -100,6 +100,11 @@ def read_config(self, config):
"to work"
)
+ if "url_preview_ip_range_whitelist" in config:
+ self.url_preview_ip_range_whitelist = IPSet(
+ config["url_preview_ip_range_whitelist"]
+ )
@NegativeMjark

NegativeMjark May 4, 2016 • edited

Contributor

Would it make sense to store the empty IPSet if the config key is missing?
I think you could then remove all the conditionals on having a whitelist from the spider code.
@NegativeMjark
Contributor

NegativeMjark commented May 4, 2016

@ara4n I think either one of my suggestions would make the tests pass. I don't think I have any further comments.

NegativeMjark added some commits May 16, 2016

@NegativeMjark
Merge branch 'develop' into matthew/preview_url_ip_whitelist
dd95eb4
@NegativeMjark
Clean up the blacklist/whitelist handling. 
Always set the config key with an empty list, even if a list isn't specified.
This means that the codepaths are the same for both the empty list and
for a missing key. Since the behaviour is the same for both cases this
makes the code somewhat easier to reason about.

All checks have passed

8 successful checks
@matrixbot
Flake8 + Packaging (Commit) — Build #682 origin/matthew/preview_url_ip_whitelist succeeded in 30 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #663 origin/matthew/preview_url_ip_whitelist succeeded in 4 min 49 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #670 origin/matthew/preview_url_ip_whitelist succeeded in 3 min 35 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #726 origin/matthew/preview_url_ip_whitelist succeeded in 1 min 15 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
eb79110
@ara4n
Owner

ara4n commented May 16, 2016

lgtm, thanks for fixing it up for me; apologies that it got lost during the main vec delivery.

@ara4n ara4n merged commit 2d98c96 into develop May 16, 2016
8 checks passed

8 checks passed

Flake8 + Packaging (Commit) Build #682 origin/matthew/preview_url_ip_whitelist succeeded in 30 sec
Details
Flake8 + Packaging (Merged PR) Build finished.
Details
Sytest Postgres (Commit) Build #663 origin/matthew/preview_url_ip_whitelist succeeded in 4 min 49 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #670 origin/matthew/preview_url_ip_whitelist succeeded in 3 min 35 sec
Details
Sytest SQLite (Merged PR) Build finished.
Details
Unit Tests (Commit) Build #726 origin/matthew/preview_url_ip_whitelist succeeded in 1 min 15 sec
Details
Unit Tests (Merged PR) Build finished.
Details

@richvdh richvdh deleted the matthew/preview_url_ip_whitelist branch Dec 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment