Repository navigation
Bugfix/review findings - #258
Draft
danthedaniel wants to merge 10 commits into
Draft
danthedaniel wants to merge 10 commits into
danthedaniel wants to merge 10 commits into
Conversation
creesch
reviewed
Sep 23, 2026
|
|
||
| String host = ctx.header("Host"); | ||
| if (host == null || host.isEmpty()) { | ||
| if (host == null || !LOCAL_HOST.matcher(host).matches()) { |
Collaborator
There was a problem hiding this comment.
Why are we suddenly blocking network access here?
Member
Author
There was a problem hiding this comment.
Not ready for review. I pushed the branch just as a backup. This is more so just a large set of potential fixes proposed by LLMs.
Member
Author
There was a problem hiding this comment.
First I need to review it. But I didn't feel like getting into that right now. My current LLM workflow usually involves uploading to a git forge to review code on there. Github's PR UI is designed for reviewing code better than looking at diffs in a terminal or text editor.
The Origin check only compared Origin against Host, so a hostname rebound to 127.0.0.1 satisfied it (DNS rebinding). When the server is bound to loopback, also require Host to be localhost, 127.x or [::1]. With LAN access enabled users may use any hostname, so only the Origin/Host comparison applies there.
Setting a status and returning from a Javalin before-handler does not stop the pipeline; the static file handler still ran and overwrote the response. Throw the matching HttpResponseException instead.
Any parse error, missing type or bad payload propagated out of onMessage, where Javalin closes the session. Catch and log instead, and reject history requests with a non-positive limit up front since they would hit historyMessages.remove(-1).
createPlayerList returns null whenever the client has no connection, which the level-only guards on connect and on tick do not cover. gson.toJson(null) then produced "null" on the wire.
A message made only of stripped characters (section signs, zero-width
spaces) passed the raw emptiness check and reached sendChat("").
Every inbound WebSocket frame and every history row's translations JSON was written to the game log at INFO level.
The list rendered five matches but Tab/arrow navigation wrapped over every match, so the highlight vanished and Enter inserted a name that was never shown.
The mousedown default action moved focus off the textarea after the name was inserted, forcing a click back into the input.
A drag starting inside the content box and released over the backdrop fires its click on the container, bypassing the stopPropagation guard and wiping the selection. Checking the target makes that guard, and the content element, unnecessary.
danthedaniel
force-pushed
the
bugfix/review-findings
branch
from
September 23, 2026 06:10
8ccfc07 to
f4b9edd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.