Skip to content

Bugfix/review findings - #258

Draft
danthedaniel wants to merge 10 commits into
mainfrom
bugfix/review-findings
Draft

danthedaniel wants to merge 10 commits into
mainfrom
bugfix/review-findings

Conversation

@danthedaniel

Copy link
Copy Markdown
Member

No description provided.


String host = ctx.header("Host");
if (host == null || host.isEmpty()) {
if (host == null || !LOCAL_HOST.matcher(host).matches()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why are we suddenly blocking network access here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.

@danthedaniel danthedaniel Sep 23, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants