Skip to content

Only allow http and https#4457

Merged
TarLaboratories merged 15 commits into1.20.1from
tar/fix-file-reading
Jan 15, 2026
Merged

Only allow http and https#4457
TarLaboratories merged 15 commits into1.20.1from
tar/fix-file-reading

Conversation

@TarLaboratories
Copy link
Copy Markdown
Contributor

What

Only allow http and https in central monitor

@TarLaboratories TarLaboratories added the type: bugfix General bug fixes label Jan 10, 2026
@TarLaboratories TarLaboratories requested a review from a team as a code owner January 10, 2026 09:21
@TarLaboratories TarLaboratories added 1.20.1 Release: Patch - 0.0.X Smaller changes that either are bug fixes or very minor tweaks. labels Jan 10, 2026
@github-actions github-actions bot added the Tests: Passed Game Tests have passed on this PR label Jan 10, 2026
@pau101
Copy link
Copy Markdown

pau101 commented Jan 10, 2026

Though this is an improvement, I would be weary of the GTCEu.isClientSide() case, this is true for a LAN host. Also, I would impose a restriction on the amount of bytes read from the stream. Here is possible patch how I may approach it, moving all logic to the client with some additional enhancements.

@TarLaboratories
Copy link
Copy Markdown
Contributor Author

It is true for a LAN host, but if you've got someone on your LAN who wants to hack you I'm pretty sure you've got bigger problems than this :)
I really don't want to move image requesting to client-side due to certain image hosts (imgur) being banned in certain regions (UK), and, as a consequence, some players not being able to actually see said images.

@pau101
Copy link
Copy Markdown

pau101 commented Jan 10, 2026

Well the thing with LAN is you have no control over who can join without additional mods. As well, LAN is used with mods like e4mc to open it up to the internet. The impact is severe enough, it at least would need a significant user warning to have that feature enabled. A LAN network may not be something you have control over in shared living spaces, though less likely to be a target I would not imagine a user to be comfortable knowing of that possibility.

For still running on server side, that can be alright if fully safe guarded with an option to disable. As well as some additional notices about importance of those allowed domains to prevent any local network web servers being inadvertently accessible.

if (!allowedProtocol) return NULL_MARKER;
boolean allowedDomain = singleplayer;
for (String domain : ConfigHolder.INSTANCE.gameplay.allowedDomains) {
if (url.getHost().equalsIgnoreCase(domain)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this use regular equals()?

@TarLaboratories TarLaboratories merged commit 3dca9ef into 1.20.1 Jan 15, 2026
4 checks passed
@TarLaboratories TarLaboratories deleted the tar/fix-file-reading branch January 15, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.20.1 Release: Patch - 0.0.X Smaller changes that either are bug fixes or very minor tweaks. Tests: Passed Game Tests have passed on this PR type: bugfix General bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants