Skip to content

fix: crash in ProtocolLogin (#4902)#4926

Merged
MillhioreBT merged 1 commit intootland:masterfrom
gesior:4902-protocollogin-crash
Jun 5, 2025
Merged

fix: crash in ProtocolLogin (#4902)#4926
MillhioreBT merged 1 commit intootland:masterfrom
gesior:4902-protocollogin-crash

Conversation

@gesior
Copy link
Contributor

@gesior gesior commented Jun 4, 2025

Pull Request Prelude

  • I have followed [proper The Forgotten Server code styling][code].
  • I have read and understood the [contribution guidelines][cont] before making this PR.
  • I am aware that this PR may be closed if the above-mentioned criteria are not fulfilled.

Changes Proposed

ProtocolLogin::getCharacterList reads IP from getConnection() in dispatcher event, but does not check, if connection still exists.
Without connection, whole login process can be skipped, as there is no way to return list of characters to Tibia Client.

Fixes crash reported in #4902

@gesior
Copy link
Contributor Author

gesior commented Jun 4, 2025

My PR fails on code style, because useless - there is no case in TFS code that could crash Game::addGuild code - PR #4908 was merged with 2 spaces indent (using spaces, not TABs) :(

#4924 fixes code style in Game::addGuild:
https://github.com/otland/forgottenserver/pull/4924/files#diff-0bb6c7b942a6c15255f935db679c066dfce4d4114fdb99698c1e1a09b7f5a98a

@ArturKnopik
Please check code style check before merging future PRs. One merged with errors makes all next PRs fail on code style.

@MillhioreBT
Copy link
Contributor

My PR fails on code style, because useless - there is no case in TFS code that could crash Game::addGuild code - PR #4908 was merged with 2 spaces indent (using spaces, not TABs) :(

#4924 fixes code style in Game::addGuild: https://github.com/otland/forgottenserver/pull/4924/files#diff-0bb6c7b942a6c15255f935db679c066dfce4d4114fdb99698c1e1a09b7f5a98a

@ArturKnopik Please check code style check before merging future PRs. One merged with errors makes all next PRs fail on code style.

I think the builds are broken anyway, it doesn't matter if you fix the code format in that place.

@MillhioreBT MillhioreBT added the priority: low Issues with this label won’t have the immediate focus label Jun 4, 2025
Copy link
Contributor

@MillhioreBT MillhioreBT left a comment

Choose a reason for hiding this comment

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

Regarding this PR, it's very rare for someone to suffer from this issue, since the old protocol is supposedly no longer used. Now, the HTTP protocol is used to fetch the character list, but I still think this IF statement is fine there.

@MillhioreBT MillhioreBT merged commit c8f0625 into otland:master Jun 5, 2025
2 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: low Issues with this label won’t have the immediate focus

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants