search

etesync / server

The Etebase server (so you can run your own)
https://www.etesync.com
GNU Affero General Public License v3.0
1.51k stars 75 forks source link

Not compatible with newer version of FastAPI #112

Closed PapaTutuWawa closed 2 years ago

PapaTutuWawa commented 2 years ago

This is not a feature request but a hint for anyone else who might be in the same situation as me:

I recently wanted to run the master branch of the EteSync server on my NixOS unstable host. However, whenever I wanted to log in or register, I got returned a 422 Unprocessable Entity. After some digging, I found out that NixOS unstable ships FastAPI version 0.70.0, while the EteSync server requires 0.63.0. This means that FastAPI has made a change somewhere between 0.63.0 and 0.70.0 that break the EteSync server, which meant that I have to downgrade the FastAPI version on NixOS.

tasn commented 2 years ago

Oh, any idea what's the diff? Will fix it ASAP.

PapaTutuWawa commented 2 years ago

So, a small investigation of mine shows that the issue starts with FastAPI version 0.65.2. What might be interesting here are the breaking/security changes in 0.65.2 and 0.65.3:

As I have no experience with FastAPI, I don't know how far digging deeper will lead me, but I will probably try.

tasn commented 2 years ago

These both sound very likely! I guess our libraries may not always set the correct content type (maybe). Which clients are failing? All of them?

PapaTutuWawa commented 2 years ago

I tried this with the EteSync app from F-Droid and the python library

felschr commented 2 years ago

@PapaTutuWawa Do you still have this issue? etebase-server works fine for me on latest nixos-unstable but I've also never had this issue before. My config for reference (I'm using it behind nginx): https://gitlab.com/felschr/nixos-config/-/blob/main/services/etebase.nix

PapaTutuWawa commented 2 years ago

@felschr The problem is that nixos-unstable does not have the master branch of etebase-server packaged, which was needed for my patched version adding LDAP support. Just for clarification: I tested this both with and without my patches. Both times the same result.

tasn commented 2 years ago

Do we need to make a new relesae?

c7hm4r commented 2 years ago

Since e0010f21f6e894380c07ecca2419cea24fae3718 I also get 422 errors, which cause the Android client to make error notifications. In that commit FastAPI is upgraded from 0.63.0 to 0.75.0. The issue does not appear on my Debian 10/buster with FastAPI versions 0.63.0, 0.65.0, 0.65.1, but with 0.65.2, 0.65.3 and 0.75.0. That is consistent with @PapaTutuWawa's finding.

tasn commented 2 years ago

Oh damn. @victor-rds, I think we should downgrade fastapi until we get to the bottom of this. Because we use msgpack this can be a pain to fix.

tasn commented 2 years ago

I'll also attempt to quick-fix it later today or sometime tomorrow.

victor-rds commented 2 years ago

Oh damn. @victor-rds, I think we should downgrade fastapi until we get to the bottom of this. Because we use msgpack this can be a pain to fix.

Yeah I tested only with the official web client, and haven't noticed any 4xx errors on my logs I will make a new PR.

PapaTutuWawa commented 2 years ago

Just wanted to say that this fixed my issue. Thanks!

tasn commented 2 years ago

Yay, thanks for letting us know!