Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions Ruby/ReadMe.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,25 @@ Attempt all SLP protocols, disable debug mode, and disable DNS SRV resolution:
ms = MineStat.new("minecraft.frag.land", 25565, 3, MineStat::Request::SLP, false, false)
```

### JSON status protocol selection

Choose a reason for hiding this comment

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

medium

The heading "JSON status protocol selection" is a bit verbose. Consider shortening it to "JSON Protocol Selection" or similar for conciseness.


Use `status_protocol` when you need to control the protocol sent in the >=1.7 JSON status handshake:

```ruby
# Explicit protocol
ms = MineStat.new("frag.land", 25565, request_type: MineStat::Request::JSON, status_protocol: 774)

# Auto/default mode (same as nil)
ms = MineStat.new("frag.land", 25565, request_type: MineStat::Request::JSON, status_protocol: :auto)
```

After polling:
- `requested_protocol`: protocol sent in handshake
- `response_protocol`: protocol from `version.protocol` in response JSON (if present)
- `protocol_mismatch`: whether request/response protocol values differ

In proxy stacks (Velocity/Bungee) and ViaVersion setups, `version.protocol` may be rewritten or shaped by requester protocol. Do not assume `response_protocol` is always the backend server's real version.

Choose a reason for hiding this comment

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

medium

This note is very important and is also present in the code comments. Consider making it more prominent, perhaps with a warning icon or a bolded "Important Note:" prefix, to draw more attention to it.


### Support
* Discord: https://discord.frag.land
* GitHub: https://github.com/FragLand/minestat
58 changes: 56 additions & 2 deletions Ruby/lib/minestat.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ class MineStat
# Default TCP/UDP timeout in seconds
DEFAULT_TIMEOUT = 5

# Default protocol version used for JSON status handshakes in auto mode.
# This preserves the current behavior while keeping future protocol bumps centralized.
DEFAULT_JSON_STATUS_PROTOCOL = 760
Comment on lines +53 to +55

Choose a reason for hiding this comment

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

medium

The comment for DEFAULT_JSON_STATUS_PROTOCOL is good, but it could be slightly more concise. The phrase "This preserves the current behavior while keeping future protocol bumps centralized" is a bit long for a constant's comment.

  # Default protocol version used for JSON status handshakes in auto mode.
  # Centralizes protocol version for future updates while maintaining backward compatibility.


# Bedrock/Pocket Edition packet offset in bytes (1 + 8 + 8 + 16 + 2)
# Unconnected pong (0x1C) = 1 byte
# Timestamp as a long = 8 bytes
Expand Down Expand Up @@ -128,6 +132,8 @@ module Request
# @param timeout [Integer] TCP/UDP timeout in seconds
# @param request_type [Request] Protocol used to poll a Minecraft server
# @param debug [Boolean] Enable or disable error output
# @param status_protocol [Integer, Symbol, nil] JSON status handshake protocol
# (`Integer` to force a specific value, `:auto`/`nil` to use the default)
# @return [MineStat] A MineStat object
# @example Simply connect to an address
# ms = MineStat.new("frag.land")
Expand Down Expand Up @@ -159,6 +165,9 @@ def initialize(address, port, options = {})
@player_list # list of players (UT3/GS4 query only)
@plugin_list # list of plugins (UT3/GS4 query only)
@protocol # protocol level
@requested_protocol # protocol used in JSON status handshake
@response_protocol # protocol returned by JSON status response
@protocol_mismatch # protocol mismatch between request and response?
Comment on lines +168 to +170

Choose a reason for hiding this comment

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

medium

The comments for @requested_protocol, @response_protocol, and @protocol_mismatch are good, but they could be aligned for better readability, similar to other instance variables above.

    @protocol                  # protocol level
    @requested_protocol        # protocol used in JSON status handshake
    @response_protocol         # protocol returned by JSON status response
    @protocol_mismatch         # protocol mismatch between request and response?

@json_data # JSON data for 1.7 queries
@latency # ping time to server in milliseconds
# TCP/UDP timeout
Expand All @@ -168,6 +177,11 @@ def initialize(address, port, options = {})
@request_type = options[:request_type] || Request::NONE
@connection_status # status of connection ("Success", "Fail", "Timeout", or "Unknown")
@try_all = false # try all protocols?
# JSON status protocol mode (:auto or Integer)
@status_protocol = normalize_status_protocol(options[:status_protocol])
@requested_protocol = nil
@response_protocol = nil
@protocol_mismatch = false
# debug mode
@debug = options[:debug].nil? ? false : options[:debug]
# enable SRV resolution?
Expand Down Expand Up @@ -605,6 +619,10 @@ def extended_legacy_request()
# {'players': {'max': 20, 'online': 0},
# 'version': {'protocol': 404, 'name': '1.13.2'},
# 'description': {'text': 'A Minecraft Server'}}
#
# The response protocol is not always the real backend server protocol.
# Proxy stacks such as Velocity/Bungee may shape status responses from the
# requester protocol, and ViaVersion may rewrite `version.protocol`.
Comment on lines +622 to +625

Choose a reason for hiding this comment

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

medium

This comment block is very important for understanding the response_protocol behavior. It's good that it's included here and in the README. Consider using a consistent phrasing for this note across both locations.

# @return [Retval] Return value
# @since 0.3.0
# @see https://wiki.vg/Server_List_Ping#Current_.281.7.2B.29
Expand All @@ -616,7 +634,8 @@ def json_request()
return retval unless retval == Retval::SUCCESS
# Perform handshake
payload = pack_varint(0)
payload << pack_varint(760)
@requested_protocol = selected_status_protocol
payload << pack_varint(@requested_protocol)
payload += [@srv_succeeded ? @srv_address.length : @address.length].pack('c') << (@srv_succeeded ? @srv_address : @address)
payload += [@srv_succeeded ? @srv_port : @port].pack('n')
payload += "\x01"
Expand All @@ -636,7 +655,12 @@ def json_request()
json_data = JSON.parse(json_data)
@online = true
@json_data = json_data
@protocol = json_data['version']['protocol'].to_i
@response_protocol = nil
if json_data['version'].is_a?(Hash) && !json_data['version']['protocol'].nil?
@response_protocol = json_data['version']['protocol'].to_i
end
@protocol = @response_protocol
@protocol_mismatch = !@response_protocol.nil? && @requested_protocol != @response_protocol
@version = json_data['version']['name']
@motd = json_data['description']
strip_motd()
Expand Down Expand Up @@ -722,6 +746,25 @@ def pack_varint(value)
end
private :pack_varint

# Normalizes JSON status protocol selection mode
# @param protocol [Integer, Symbol, nil] Protocol selection value
# @return [Integer, Symbol] Integer protocol or :auto
def normalize_status_protocol(protocol)
return :auto if protocol.nil? || protocol == :auto
return protocol if protocol.is_a?(Integer)
Comment on lines +752 to +754

Choose a reason for hiding this comment

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

medium

The normalize_status_protocol method's logic is clear. However, the return protocol if protocol.is_a?(Integer) line could be combined with the raise ArgumentError to make the intent of the if statement clearer: if it's not nil or :auto, it must be an Integer.

    return :auto if protocol.nil? || protocol == :auto
    raise ArgumentError, "status_protocol must be an Integer, :auto, or nil" unless protocol.is_a?(Integer)
    protocol

Choose a reason for hiding this comment

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

P2 Badge Reject out-of-range status_protocol values

normalize_status_protocol currently accepts any Integer, but pack_varint later truncates to 32 bits (value &= 0xFFFFFFFF), so large values are silently rewritten on the wire while requested_protocol still exposes the original input. In practice, passing something like 1 << 40 sends protocol 0, which can produce incorrect status responses and misleading mismatch metadata instead of honoring the caller’s explicit protocol request.

Useful? React with 👍 / 👎.


raise ArgumentError, "status_protocol must be an Integer, :auto, or nil"
end
private :normalize_status_protocol

# Resolves the protocol to use for JSON status handshake
# @return [Integer] Protocol version for the handshake
def selected_status_protocol
return DEFAULT_JSON_STATUS_PROTOCOL if @status_protocol == :auto
@status_protocol
end
private :selected_status_protocol

# Bedrock/Pocket Edition (unconnected ping request)
# @note
# 1. Client sends:
Expand Down Expand Up @@ -907,6 +950,17 @@ def query_request()
# @note This is arbitrary and varies by Minecraft version (may also be shared by multiple Minecraft versions)
attr_reader :protocol

# Protocol used in the JSON status handshake request
# @note `:auto`/`nil` mode uses {DEFAULT_JSON_STATUS_PROTOCOL}
attr_reader :requested_protocol

# Protocol observed in JSON status response (`version.protocol`)
# @note In proxy environments (Velocity/Bungee/ViaVersion), this may not match backend server protocol
attr_reader :response_protocol

# Whether requested and response protocols are different
attr_reader :protocol_mismatch
Comment on lines +953 to +962

Choose a reason for hiding this comment

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

medium

The attr_reader comments for requested_protocol, response_protocol, and protocol_mismatch are well-written and provide good context. Ensure that the spacing for these new attr_reader definitions is consistent with the existing ones for better readability.

  attr_reader :protocol

  # Protocol used in the JSON status handshake request
  # @note `:auto`/`nil` mode uses {DEFAULT_JSON_STATUS_PROTOCOL}
  attr_reader :requested_protocol

  # Protocol observed in JSON status response (`version.protocol`)
  # @note In proxy environments (Velocity/Bungee/ViaVersion), this may not match backend server protocol
  attr_reader :response_protocol

  # Whether requested and response protocols are different
  attr_reader :protocol_mismatch


# Complete JSON response data
# @note Received using SLP 1.7 (JSON) queries
# @since 0.3.0
Expand Down
159 changes: 159 additions & 0 deletions Ruby/test/test_status_protocol.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
require 'json'
require 'minitest/autorun'
require_relative '../lib/minestat'

class FakeJsonSocket
attr_reader :writes

def initialize(json_payload)
@writes = []
@read_buffer = +""
@recv_buffer = json_payload.dup

packet_id = pack_varint(0)
json_len = pack_varint(json_payload.bytesize)
total_len = pack_varint(packet_id.bytesize + json_len.bytesize + json_payload.bytesize)

@read_buffer << total_len
@read_buffer << packet_id
@read_buffer << json_len
end

def write(data)
@writes << data
data.bytesize
end

def flush
end

def close
end

def read(length)
return nil if @read_buffer.empty?

chunk = @read_buffer.byteslice(0, length)
@read_buffer = @read_buffer.byteslice(length..-1) || +""
chunk
end

def recv(length, _flags = nil)
chunk = @recv_buffer.byteslice(0, length) || +""
@recv_buffer = @recv_buffer.byteslice(length..-1) || +""
chunk
end

private

def pack_varint(value)
value &= 0xFFFFFFFF
buf = +""
loop do
byte = value & 0x7F
value >>= 7
if value != 0
buf << (byte | 0x80).chr
else
buf << byte.chr
break
end
end
buf
end
end

class MineStatProtocolHarness < MineStat
attr_reader :captured_socket

def initialize(options = {}, json_response_protocol: 760)
@json_response_protocol = json_response_protocol
super('example.com', 25565, {
timeout: 1,
request_type: MineStat::Request::JSON,
srv_enabled: false,
debug: false
}.merge(options))
end

private

def resolve_a
true
end

def resolve_srv
false
end

def connect
payload = {
'version' => { 'name' => '1.21.1', 'protocol' => @json_response_protocol },
'players' => { 'online' => 1, 'max' => 20 },
'description' => { 'text' => 'Hello world' }
}
@captured_socket = FakeJsonSocket.new(JSON.generate(payload))
@server = @captured_socket
MineStat::Retval::SUCCESS
end
end

class MineStatStatusProtocolTest < Minitest::Test
def decode_varint(data, offset = 0)

Choose a reason for hiding this comment

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

medium

The test class name MineStatStatusProtocolTest is descriptive. However, for consistency with Ruby's naming conventions, it's common to use TestMineStatStatusProtocol or MineStat::StatusProtocolTest.

result = 0
shift = 0
consumed = 0

loop do
byte = data.getbyte(offset + consumed)
raise 'Invalid varint stream' if byte.nil?

result |= (byte & 0x7F) << shift
consumed += 1
break if (byte & 0x80).zero?

shift += 7
end

[result, consumed]
end

def extract_requested_protocol(handshake_packet)
_packet_len, packet_len_size = decode_varint(handshake_packet, 0)
packet_id = handshake_packet.getbyte(packet_len_size)
raise "Unexpected packet id: #{packet_id}" unless packet_id == 0
Comment on lines +123 to +124

Choose a reason for hiding this comment

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

medium

The raise statement here is good for ensuring the packet ID is as expected. However, in a test helper, it might be more robust to return nil or raise a more specific test-related error, or even assert the packet ID directly in the test method if it's critical for the test's premise.


protocol, = decode_varint(handshake_packet, packet_len_size + 1)
protocol
end

def test_explicit_status_protocol_is_sent_in_handshake
ms = MineStatProtocolHarness.new({ status_protocol: 774 }, json_response_protocol: 774)

handshake_packet = ms.captured_socket.writes[0]
assert_equal 774, extract_requested_protocol(handshake_packet)
assert_equal 774, ms.requested_protocol
assert_equal 774, ms.response_protocol
assert_equal false, ms.protocol_mismatch
end

def test_auto_mode_uses_default_for_auto_and_nil
ms_auto = MineStatProtocolHarness.new({ status_protocol: :auto }, json_response_protocol: 760)
ms_nil = MineStatProtocolHarness.new({ status_protocol: nil }, json_response_protocol: 760)

assert_equal 760, extract_requested_protocol(ms_auto.captured_socket.writes[0])
assert_equal 760, extract_requested_protocol(ms_nil.captured_socket.writes[0])
assert_equal 760, ms_auto.requested_protocol
assert_equal 760, ms_nil.requested_protocol
end

def test_mismatch_records_metadata_without_raising_error
ms = MineStatProtocolHarness.new({ status_protocol: 760 }, json_response_protocol: 774)

assert_equal true, ms.online
assert_equal 'Success', ms.connection_status
assert_equal 760, ms.requested_protocol
assert_equal 774, ms.response_protocol
assert_equal true, ms.protocol_mismatch
end
end