feelfreelinux / cspot

A Spotify Connect player targeting, but not limited to embedded devices (ESP32).
Other
460 stars 43 forks source link

addUserHandler does not work anymore #134

Closed philippe44 closed 1 year ago

philippe44 commented 1 year ago

When receiving a request to add a new user, the handler here https://github.com/feelfreelinux/cspot/blob/1e538aeea3487a13949a44fda1fa346ddc61c66c/cspot/src/ZeroconfAuthenticator.cpp#L51 does not receive the body of the request, where the blob is. It then crashes as it tries to parse an empty body. I don't know why, but here https://github.com/feelfreelinux/bell/blob/e2b5d051328dbee2b6b38560cd2fa1c6055da1ab/src/HTTPServer.cpp#L489 the body is not copied (I've logged that, on Linux and Windows)

philippe44 commented 1 year ago

Got it - int the loop that fills the body with the HTTP request, it uses body.data() + read which does not increment the "size" attribute of body. So we end up with a std::string that has reserved space but an actual size of 0, so everything after that goes south. I will add a fix to my PR

latonita commented 1 year ago

@philippe44 did you manage to fix this issue? After 4 hours of trying to select proper library versions I finally built it for esp32 and see same issue as you do.

philippe44 commented 1 year ago

Yes, see the PR I've pushed or my own repo

latonita commented 1 year ago

@philippe44 apologies, i'm not sure i know how to find your PR :) can you please post a link to a fix- PR or place in your repo.

Basically, if i pull from your repo - the fix is there and i can try build it, right? :)

latonita commented 1 year ago

I mean I see your change https://github.com/feelfreelinux/cspot/commit/7254726892f74d5acb998d8c8dd6d38cd0c072b3 and I use this version, however it still crashes, since request->body is empty

philippe44 commented 1 year ago

No it's here https://github.com/feelfreelinux/bell/blob/b0b770ed7f451f27f8eec8484fb14604c4fb9283/src/HTTPServer.cpp#L471 (and a few lines below with the 2nd resize). You can try to pull everything from my repo, but unless you just want to build clispot, I'd advise doing just the small change as I've added many other things

latonita commented 1 year ago

@philippe44 thanks a lot! Its working now.