Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update encoder.rb to limit hex false positives #286

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RogerPodacter
Copy link
Contributor

Looking at Util.prefixed? arg to determine whether to treat something as hex makes it impossible to encode anything that starts with the bytes [48,120], which is not that infrequent.

Also, I don't think we get any benefit from this part of the conditional anyway since if the string isn't both hex chars and 2x the input size things will fail anyway.

Longer term it might be beneficial to force the user to specify the encoding.

Thanks!

@q9f
Copy link
Owner

q9f commented Aug 28, 2024

Longer term it might be beneficial to force the user to specify the encoding.

Or we will implement actual types in Ruby 🤡

Will run the tests once I'm back in the office.

@RogerPodacter
Copy link
Contributor Author

Thanks!! And would types really be so crazy here? The hex v. bytes ambiguity is just so brutal for our use-case!

Should we force everyone to use a special class? Definitely annoying but maybe worth it to have a decent night's sleep?

Here's what the AI spit out. Thoughts?

class ByteString
  attr_reader :bytes

  # Factory methods
  def self.from_hex(hex_string)
    # Remove leading '0x' if present
    hex_string = hex_string[2..] if hex_string.start_with?('0x')

    # Ensure it's a valid hex string and has an even number of characters
    raise ArgumentError, "Invalid hex string" unless hex_string.match?(/\A[\h]*\z/)
    raise ArgumentError, "Hex string length must be even" unless hex_string.length.even?

    new([hex_string].pack('H*'))
  end

  def self.from_bin(binary_string)
    raise ArgumentError, "Binary string expected with ASCII-8BIT encoding" unless binary_string.is_a?(String) && binary_string.encoding == Encoding::ASCII_8BIT
    new(binary_string)
  end

  # Convert back to hex
  def to_hex
    @bytes.unpack1('H*')
  end

  # Convert to binary
  def to_bin
    @bytes
  end

  # Prevent calling to_s to avoid ambiguity
  def to_s
    raise NoMethodError, "Ambiguous conversion: Use `to_bin` for bytes or `to_hex` for hex representation"
  end

  # For debugging or string representation
  def inspect
    "#<ByteString hex=#{to_hex} length=#{@bytes.bytesize}>"
  end

  # Equality comparison
  def ==(other)
    other.is_a?(ByteString) && @bytes == other.bytes
  end

  # More strict equality comparison (used by Hash)
  def eql?(other)
    self == other
  end

  # Hash method to allow use in sets or as hash keys
  def hash
    @bytes.hash
  end

  private_class_method :new

  # Initialize with binary string (bytes)
  def initialize(binary_string)
    raise ArgumentError, "Binary string must have ASCII-8BIT encoding" unless binary_string.encoding == Encoding::ASCII_8BIT
    @bytes = binary_string.dup.freeze
  end
end

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