Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix missprint (or CopyPaste) that brake push redact_content setting. #2422

Closed
wants to merge 1 commit into from

Conversation

slipeer
Copy link
Contributor

@slipeer slipeer commented Aug 18, 2017

Fix missprint (or CopyPaste) that brake push: redact_content setting.
There was mistake in config section name.

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

2 similar comments
@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@slipeer slipeer changed the title Fix missprint (or CopyPaste) that brake push: redact_content setting. Fix missprint (or CopyPaste) that brake push redact_content setting. Aug 21, 2017
@dbkr
Copy link
Member

dbkr commented Oct 19, 2017

We now support this properly, ie. the clients can say whether they want event content to be sent in the push. Given this, and the fact that this has never worked, perhaps it would be better to just remove this?

@krombel
Copy link
Contributor

krombel commented Oct 19, 2017

  1. It might have worked - in cases where admins where setting it right.
  2. The server owner might want to enforce this setting.

As the overhead of this implementation should not really impact the overall operation I would just fix the config.

@Tokodomo
Copy link

I'm all with you (@krombel). I don't want to rely on the clients in terms of privacy protection. Sending the event content in the push and exposing it to Google or Apple and even to the push gateway hosted by matrix.org shouldn't be an option at all! This is an easy fix!

@Tokodomo
Copy link

@dbkr Can you please merge this pull request?

dbkr added a commit that referenced this pull request Nov 8, 2017
The redact_content option never worked because it read the wrong config
section. The PR introducing it
(#2301) had feedback suggesting the
name be changed to not re-use the term 'redact' but this wasn't
incorporated.

This reanmes the option to give it a less confusing name, and also
means that people who've set the redact_content option won't suddenly
see a behaviour change when upgrading synapse, but instead can set
include_content if they want to.

This PR also updates the wording of the config comment to clarify
that this has no effect on event_id_only push.

Includes #2422
@dbkr
Copy link
Member

dbkr commented Nov 8, 2017

I've merged this into #2650 - hopefully this is good for everyone?

@richvdh
Copy link
Member

richvdh commented Feb 13, 2018

it's good for me.

@richvdh richvdh closed this Feb 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants