Fix WKS record protocol field encoding.

[donapieppo]

Fix WKS record protocol field encoding and decoding size according to RFC 1035 Section 3.4.2 where PROTOCOL is an 8 bit.
I spotted the errror using Dnsruby::ZoneTransfer where i got the error

Dnsruby::MessageDecoder#assert_buffer_position_valid': requested position of 1871 must be     
  between 0 and buffer size (1870). (Dnsruby::DecodeError) 

The patch just change the pack format from "n" to "C" in lib/dnsruby/resource/IN.rb

[keithrbennett]

Hi, I have no knowledge of this particular domain, but asked Claude AI about it, and had its response reviewed successfully by Qwen AI. TL/DR is that the fix is correct, and in addition this would be a good opportunity to add unit tests for the feature. Suggested test code is also included in the response:

https://claude.ai/share/e0d07f8a-f26b-4548-a0db-71b12a7ee817

(EDIT: The source code was not included in the text referred to by the link; I have pasted it in the comment following this one.)

Qwen review at: https://chat.qwen.ai/s/58977257-7fe0-4b11-8307-c4d0196c2a69?fev=0.2.64

@alexdalitz I realize some people would prefer not to receive comments like these where I sanity checked the information but did not deeply verify it. If you would prefer me not post similar posts in the future, just say so and I will refrain.

[keithrbennett]

Sorry, I could not get a link to the conversation that included the test code. Here it is, with explanation following:

# test/tc_wks.rb
require_relative 'spec_helper'

class WKSTest < Minitest::Test
  # Protocol 6 = TCP, per IANA; fits in 8 bits (0-255).
  # Bitmap marks ports 80 (HTTP) and 443 (HTTPS) as available.
  ADDR     = "192.0.2.1"
  PROTOCOL = 6
  BITMAP   = "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02".b

  def wks
    Dnsruby::RR::IN::WKS.new(
      Dnsruby::IPv4.new(ADDR),
      PROTOCOL,
      BITMAP
    )
  end

  def encode(rr)
    msg = Dnsruby::MessageEncoder.new
    rr.encode_rdata(msg)
    msg.to_s
  end

  def roundtrip(rr)
    encoded = encode(rr)
    # The encoded wire format for WKS is: 4 bytes address + 1 byte protocol + bitmap.
    # With the old "n" (2-byte) format this was 6 + bitmap; now it must be 5 + bitmap.
    assert_equal 4 + 1 + BITMAP.bytesize, encoded.bytesize,
      "wire format must be 4-byte address + 1-byte protocol + bitmap"

    decoder = Dnsruby::MessageDecoder.new(encoded)
    Dnsruby::RR::IN::WKS.decode_rdata(decoder)
  end

  def test_roundtrip_preserves_protocol
    decoded = roundtrip(wks)
    assert_equal PROTOCOL, decoded.protocol
  end

  def test_roundtrip_preserves_address
    decoded = roundtrip(wks)
    assert_equal ADDR, decoded.address.to_s
  end

  def test_roundtrip_preserves_bitmap
    decoded = roundtrip(wks)
    assert_equal BITMAP, decoded.bitmap
  end

  def test_protocol_field_is_one_byte
    # Regression: the old "n" pack format wrote 2 bytes for protocol,
    # causing MessageDecoder to overshoot the buffer by 1 byte (Dnsruby::DecodeError).
    encoded = encode(wks)
    # Byte 4 (0-indexed) must be the protocol value itself, not the high byte of a 16-bit field.
    assert_equal PROTOCOL, encoded.bytes[4]
  end

  def test_max_protocol_value
    rr = Dnsruby::RR::IN::WKS.new(Dnsruby::IPv4.new(ADDR), 255, BITMAP)
    decoded = roundtrip(rr)
    assert_equal 255, decoded.protocol
  end
end

A few notes on the choices:

test_protocol_field_is_one_byte is the regression test that would have caught the original bug — it directly asserts that byte index 4 of the wire encoding carries the protocol value, which fails with the old "n" format (which would write 0x00 there and the protocol value at byte 5).

test_roundtrip_* are the correctness tests: encode then decode and verify all three fields survive intact. Splitting them into three assertions rather than one compound one gives clearer failure messages.

test_max_protocol_value exercises the boundary (255) to confirm the 8-bit field handles the full valid range.

The BITMAP constant is intentionally non-trivial (marks real ports) to ensure bitmap content also survives the round-trip without corruption from an off-by-one in buffer consumption.

[alexdalitz]

Hey thanks!This all looks good, but I’m afraid I’ve been super busy and melting in the U.K. heatwave.I’ll try to merge this and release a new version over the weekend.Sent from my iPhoneOn 15 Jun 2026, at 12:38, Donapieppo ***@***.***> wrote:Fix WKS record protocol field encoding and decoding size according to RFC 1035 Section 3.4.2 where PROTOCOL is an 8 bit. I spotted the errror using Dnsruby::ZoneTransfer where i got the error Dnsruby::MessageDecoder#assert_buffer_position_valid': requested position of 1871 must be between 0 and buffer size (1870). (Dnsruby::DecodeError) The patch just change the pack format from "n" to "C" in lib/dnsruby/resource/IN.rb You can view, comment on, or merge this pull request online at:   #220 Commit Summary 5ca0b22 Fix WKS record protocol field encoding and decoding size according to RFC 1035 Section 3.4.2 where PROTOCOL is an 8 bit File Changes (1 file) M lib/dnsruby/resource/IN.rb (4) Patch Links: https://github.com/alexdalitz/dnsruby/pull/220.patch https://github.com/alexdalitz/dnsruby/pull/220.diff —Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today! You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>