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

Pass additional headers from the worker to the main process. #7797

Merged
merged 7 commits into from
Jul 9, 2020

Conversation

clokep
Copy link
Member

@clokep clokep commented Jul 7, 2020

I believe that this will fix #6396 by passing the X-Forwarded-For and User-Agent headers from the worker to the main process when proxying requests.

Note that a User-Agent is usually generated, but will get overridden if provided as a header:

actual_headers = {
b"Content-Type": [b"application/json"],
b"User-Agent": [self.user_agent],
b"Accept": [b"application/json"],
}
if headers:
actual_headers.update(headers)

@clokep clokep marked this pull request as ready for review July 7, 2020 14:05
@clokep
Copy link
Member Author

clokep commented Jul 7, 2020

I wasn't really sure how to test this, but I think it is doing the right thing.

@clokep clokep requested a review from a team July 7, 2020 15:12
synapse/app/generic_worker.py Outdated Show resolved Hide resolved
@erikjohnston
Copy link
Member

It might also be nice at the persistence stage to ignore/error when we try and persist ips/user agents that are our own, or something

@clokep clokep requested a review from richvdh July 8, 2020 17:51
@clokep clokep requested a review from richvdh July 8, 2020 18:51
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@clokep
Copy link
Member Author

clokep commented Jul 9, 2020

It might also be nice at the persistence stage to ignore/error when we try and persist ips/user agents that are our own, or something

We talked a little bit about this and there's a few pieces of it that would likely be worth doing separately:

  • The XForwardedForRequest should potentially filter out private IPs.
  • We should potentially not save - into the database (but this theoretically shouldn't happen anymore...)
  • User agents are a bit more problematic since they can really be any string...

For now I'm going to leave this.

@clokep clokep merged commit 8fa7fdd into develop Jul 9, 2020
@clokep clokep deleted the clokep/worker-ip branch July 9, 2020 11:34
anoadragon453 added a commit that referenced this pull request Jul 20, 2020
…anoa/amorgan.xyz

* 'release-v1.17.0' of github.com:matrix-org/synapse: (117 commits)
  1.17.0
  update changelog
  fix migration, again
  fix changelog
  1.16.1
  Drop incorrectly-added table `local_rejections_stream`. (#7816)
  1.17.0rc1
  Fix some spelling mistakes / typos. (#7811)
  `update_membership` declaration: now always returns an event id. (#7809)
  Improve stacktraces from exceptions in background processes (#7808)
  Fix `can only concatenate list (not "tuple") to list` exception (#7810)
  Pass original request headers from workers to the main process. (#7797)
  Generate real events when we reject invites (#7804)
  Add `HomeServer.signing_key` property (#7805)
  Revert "Update the installation docs on apt-transport-https (#7801)"
  Do not use simplejson in Synapse. (#7800)
  Stop passing bytes when dumping JSON (#7799)
  Update the installation docs on apt-transport-https (#7801)
  shuffle changelog slightly
  1.16.0
  ...
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit '43726783e': (22 commits)
  1.17.0rc1
  Fix some spelling mistakes / typos. (#7811)
  `update_membership` declaration: now always returns an event id. (#7809)
  Improve stacktraces from exceptions in background processes (#7808)
  Fix `can only concatenate list (not "tuple") to list` exception (#7810)
  Pass original request headers from workers to the main process. (#7797)
  Generate real events when we reject invites (#7804)
  Add `HomeServer.signing_key` property (#7805)
  Revert "Update the installation docs on apt-transport-https (#7801)"
  Do not use simplejson in Synapse. (#7800)
  Stop passing bytes when dumping JSON (#7799)
  Update the installation docs on apt-transport-https (#7801)
  shuffle changelog slightly
  Change Caddy links (old is deprecated) (#7789)
  Stop populating unused table `local_invites`. (#7793)
  Refactor getting replication updates from database v2. (#7740)
  Add libwebp dependency to Dockerfile (#7791)
  Add documentation for JWT login type and improve sample config. (#7776)
  Convert the appservice handler to async/await. (#7775)
  Don't ignore `set_tweak` actions with no explicit `value`. (#7766)
  ...
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.

Device IP & UA can get overwritten by Synapse on matrix.org
3 participants