Skip to content

Conversation

@SakuraIsayeki
Copy link
Contributor

@SakuraIsayeki SakuraIsayeki commented May 24, 2025

Overhauled the whitelist system to use a new dedicated service.
This one is memory-based to optimise lookups, rather than accessing the file at each server join for WL comparison.

Added support for in-file comments, by using # prefix (considered invalid and ignored by parser). Could come in handy for those massive files, or just keeping track of "whose IP is this" cases.
Added support for ranges using CIDR notation, to allow for entire subnets to be whitelisted. Particularly useful for LAN parties.

New default for the whitelist.txt file looks like this :

# Localhost
127.0.0.1
::1

# Uncomment to allow IPs within private ranges
# 10.0.0.0/8
# 172.16.0.0/12
# 192.168.0.0/16
# fe80::/10
# fd00::/8

Resolves #3091.

Introduces a class for managing a whitelist of IP addresses and networks, enabling adding, removing, and persisting entries to a file. Whitelist is used to allow or restrict access based on IP, with support for CIDR networks and individual IPs.
Clarifies and adds more private IP range options in the whitelist comments for better configuration guidance
Introduces the whitelist system as a core component and applies it during player IP checks, replacing direct file-based whitelisting calls with a dedicated service to improve clarity and maintainability.
Adds logic to check if the file ends with a newline and inserts one if needed to prevent line concatenation issues.
@hakusaro
Copy link
Member

@ACaiCat if you think the code is good, can you add a code review too?

Copy link
Member

@hakusaro hakusaro left a comment

Choose a reason for hiding this comment

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

I tested this and after adding 127.0.0.1 to the whitelist and using /reload, I was not able to whitelist myself.

Also, Terraria doesn't support IPv6.

@hakusaro
Copy link
Member

CleanShot 2025-05-25 at 12 00 39

@ACaiCat
Copy link
Contributor

ACaiCat commented May 25, 2025

@ACaiCat if you think the code is good, can you add a code review too?

Maybe you should find someone more professional, but it's not me.

@hakusaro
Copy link
Member

Maybe you should find someone more professional, but it's not me.

You are professional

@hakusaro
Copy link
Member

Also, if the whitelist is blank, I think it would be fine to populate it with the example whitelist instead of leaving it blank. e.g., if whitelist is disabled, and is empty, add the default config to it.

@ACaiCat
Copy link
Contributor

ACaiCat commented May 25, 2025

I tested this and after adding 127.0.0.1 to the whitelist and using /reload, I was not able to whitelist myself.

Also, Terraria doesn't support IPv6.

It seems can not be reloaded through /reload command (Need restart whole server)

Enhances the whitelist command to correctly handle parameter input, provides success or error messages based on the addition outcome.
Forgot to check if Terraria itself actually DOES support IPv6...

According to a few researches, latest hotfixes have grazed the subject. Hopefully we'll get IPv6 soon 🤞🏻
Introduces a method to reload whitelist data from file, allowing updates without restarting the server.
Copy link
Member

@sgkoishi sgkoishi left a comment

Choose a reason for hiding this comment

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

Other note:

  • There's a submodule update that actually reverts 57039ed, probably unwanted.
  • The include of .idea folder?

@SakuraIsayeki
Copy link
Contributor Author

Other note:

  • There's a submodule update that actually reverts 57039ed, probably unwanted.

  • The include of .idea folder?

Oops, indeed unwanted changes. Lemme fix that right away 😅

Oops, should've paid more attention ^^'
@SakuraIsayeki SakuraIsayeki requested a review from hakusaro May 25, 2025 15:24
Switches to bitwise AND operator to correctly combine add/remove operations with line updates.
@SakuraIsayeki SakuraIsayeki requested a review from sgkoishi May 25, 2025 16:27
@ACaiCat
Copy link
Contributor

ACaiCat commented May 30, 2025

I test it on my win server, /whitelist command can't work.I am sure that no other process was using whitelist.txt

Log:

2025-05-30 21:28:08 - Utils: INFO: Server execute: /whitelist 221.12.2.1。
2025-05-30 21:28:08 - Command: ERROR: System.IO.IOException: The process cannot access the file 'C:\Users\Administrator\Desktop\a\tshock\whitelist.txt' because it is being used by another process.
   at Microsoft.Win32.SafeHandles.SafeFileHandle.CreateFile(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options) in System.Private.CoreLib.dll:token 0x60001ce+0xa7
   at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable`1 unixCreateMode) in System.Private.CoreLib.dll:token 0x60001cd+0x6
   at System.IO.Strategies.OSFileStreamStrategy..ctor(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable`1 unixCreateMode) in System.Private.CoreLib.dll:token 0x6008cfe+0x14
   at System.IO.Strategies.FileStreamHelpers.ChooseStrategyCore(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable`1 unixCreateMode) in System.Private.CoreLib.dll:token 0x6008cd5+0xa
   at System.IO.FileInfo.OpenRead() in System.Private.CoreLib.dll:token 0x6008742+0x0
   at TShockAPI.Whitelist.AddLine(ReadOnlySpan`1 content) in /home/runner/work/TShock/TShock/TShockAPI/Whitelist.cs:line 227
   at TShockAPI.Whitelist.AddToWhitelist(IPAddress ip) in /home/runner/work/TShock/TShock/TShockAPI/Whitelist.cs:line 203
   at TShockAPI.Whitelist.AddToWhitelist(ReadOnlySpan`1 ip) in /home/runner/work/TShock/TShock/TShockAPI/Whitelist.cs:line 175
   at TShockAPI.Commands.Whitelist(CommandArgs args) in /home/runner/work/TShock/TShock/TShockAPI/Commands.cs:line 1747
   at TShockAPI.Command.Run(CommandArgs args) in /home/runner/work/TShock/TShock/TShockAPI/Commands.cs:line 158

@SakuraIsayeki
Copy link
Contributor Author

I test it on my win server, /whitelist command can't work.I am sure that no other process was using whitelist.txt

By any chance, would you have the repro steps for this one ? Because I can't repro the issue.

@ACaiCat
Copy link
Contributor

ACaiCat commented May 30, 2025

By any chance, would you have the repro steps for this one ? Because I can't repro the issue.

Just try to add a ip through /whitelist 192.168.1.2

@ACaiCat
Copy link
Contributor

ACaiCat commented May 30, 2025

@THEXN tested it too and got the same error.
c34ee5cc3b502444c2953575df78f4d8

@SakuraIsayeki
Copy link
Contributor Author

Okay. Seems I derped handling the file streams.
Now fixed, give it a go on last commit :)

@ACaiCat
Copy link
Contributor

ACaiCat commented May 30, 2025

It works now.
image

Add a warning message when IPv6 addresses are used in whitelist commands, as IPv6 support isn't implemented yet, and update error message for better clarity on correct command usage
Switches to localized strings for IPv6 warnings and invalid whitelist entry logs.
@SakuraIsayeki SakuraIsayeki requested a review from ACaiCat June 16, 2025 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow IP ranges in whitelist

4 participants