Skip to content

Refactor DbHelper connection handling#28

Open
harbor208199 wants to merge 5 commits intomichal-kapala:masterfrom
harbor208199:refactor-dbhelper
Open

Refactor DbHelper connection handling#28
harbor208199 wants to merge 5 commits intomichal-kapala:masterfrom
harbor208199:refactor-dbhelper

Conversation

@harbor208199
Copy link
Copy Markdown
Contributor

DbHelper relied on a single static SQLite connection and interpolated user data into SQL, which risks "database is locked" failures, leaked handles, and SQL injection. It also mapped UbiId incorrectly.
This refactor adds scoped connections, parameterized commands, and correct field mapping without changing any public API

  • Added a CreateConnection() helper and wrapped every DbHelper method in using blocks so each operation opens/closes its own SQLite connection and logs init failures.

  • Updated all raw SQL (privilege lookup, telemetry inserts, messaging/invite helpers) to use parameterized SQLiteCommand to avoid injection/quoting issues

  • Fixed GetUserByName / GetUserByID to populate UbiId from the ubi_id column instead of the password field.

@michal-kapala michal-kapala added the enhancement New feature or request label Nov 20, 2025
@michal-kapala michal-kapala self-requested a review November 20, 2025 12:22
@harbor208199
Copy link
Copy Markdown
Contributor Author

I dug further into the code and noticed that Invitations were never serialized because GameSessionInvitationReceived.ToBuffer still threw NotImplementedException even though GetInvitationsReceived tried to serialize each invite, every fetch would fail.
Also, accepting a friendship did no check for pending invite, any client could call "accept" with an arbitrary PID and get marked as friend. I filled in the serializer and enforces that an acceptance only succeeds when a matching pending request exists

  • Implemented GameSessionInvitationReceived.ToBuffer to write the session key, sender PID, message, and timestamp so invitation lists can be serialized and returned.

  • DbHelper.AddFriend now looks for a pending invitation between requester/requestee and only then promotes it to Friend (keeping existing details if unspecified), otherwise it fails and logs

  • FriendsService checks that result and responds with Friends_UserNotFound when there's no pending invite instead of silently auto-adding

@DeathHound6
Copy link
Copy Markdown
Collaborator

@harbor208199 I believe we've previously seen a bug where 2 users can send friend requests to each other, so we end up with the same 2 users having 2 pending invitations. If both users then accept those, they are on each other's friend lists twice
I assume this bug is (theoretically) fixed now by having AddFriend check for a pending friend request?

@IAssassinTV
Copy link
Copy Markdown
Collaborator

IAssassinTV commented Nov 21, 2025

@harbor208199 I believe we've previously seen a bug where 2 users can send friend requests to each other, so we end up with the same 2 users having 2 pending invitations. If both users then accept those, they are on each other's friend lists twice

I assume this bug is (theoretically) fixed now by having AddFriend check for a pending friend request?

Yep, that's still the case.
I think it even happens if a user receives a friend request and the other accepts it. A game restart has always fixed that. Nonetheless, it would be nice to have it solved in the first place.

@harbor208199
Copy link
Copy Markdown
Contributor Author

Exactly, that's what this change tightens up, AddFriend now only promotes when there's a real pending invite from requester to requestee, so you can't accept an arbitrary PID and each accept just flips its own pending row. That should prevent the double friend scenario you've seen, and avoid needing a restart

@DeathHound6
Copy link
Copy Markdown
Collaborator

Exactly, that's what this change tightens up, AddFriend now only promotes when there's a real pending invite from requester to requestee, so you can't accept an arbitrary PID and each accept just flips its own pending row. That should prevent the double friend scenario you've seen, and avoid needing a restart

That's awesome! I'll hopefully get a good look into these changes in a few hours

@harbor208199
Copy link
Copy Markdown
Contributor Author

harbor208199 commented Nov 21, 2025

Follow up: kept the pending invite requirement and now collapse any duplicate relationship rows into single Friend entry on acceptance

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants