Data loss when writing long lines with File#print after File#set_encoding

[bloovis]

Writing long lines to a file can cause characters to be dropped in some situations. Specifically, if I use File#set_encoding and set invalid to :skip, then File#print of a very long line causes bytes to dropped at 1K offsets.

I’m using Crystal 1.18.2 on Fedora Linux 43 (x86_64).

Here is a test program that demonstrates the problem: when I run it, it generates a file containing 2999 bytes, when it should be 3001 bytes. When I delete the set_encoding line, the problem goes away.

longline = "0123456789" * 300
File.open("junk", "w") do |f|
  f.set_encoding("UTF-8", invalid: :skip) # This causes the data loss.
  f.print(longline)
  f.print("\n")
end
size = File.info("junk").size
puts "file junk has #{size} bytes, should be 3001"

This problem also happens with IO::Memory. In the following test program the "|" characters are dropped.

longline = ("." * 1024 + "|") * 4
buffer = IO::Memory.new
buffer.set_encoding "UTF-8", invalid: :skip
buffer << longline

puts buffer

puts "buffer has #{buffer.bytesize} bytes, should be 4100"

[jgaskins]

The bug occurs in this method. As noted in the OP, the 1k offsets aren't a coincidence. They correlate with the output buffer size for encodings and when you change that buffer size, it also changes which characters are stripped, so it's happening on those boundaries.

It looks like iconv is setting errno to E2BIG in this case. This monkeypatch gets it working (see the NOTE comment):

longline = ("." * 1024 + "|") * 4
buffer = IO::Memory.new
buffer.set_encoding "UTF-8", invalid: :skip
buffer << longline

puts buffer

puts "buffer has #{buffer.bytesize} bytes, should be 4100"

class IO
  private class Encoder
    def write(io : IO, slice : Bytes) : Nil
      inbuf_ptr = slice.to_unsafe
      inbytesleft = LibC::SizeT.new(slice.size)
      outbuf = uninitialized UInt8[1024]
      while inbytesleft > 0
        outbuf_ptr = outbuf.to_unsafe
        outbytesleft = LibC::SizeT.new(outbuf.size)
        err = @iconv.convert(pointerof(inbuf_ptr), pointerof(inbytesleft), pointerof(outbuf_ptr), pointerof(outbytesleft))
        # NOTE: We're ignoring `E2BIG` set by `iconv`
        if err == Crystal::Iconv::ERROR && Errno.value != Errno::E2BIG
          @iconv.handle_invalid(pointerof(inbuf_ptr), pointerof(inbytesleft))
        end
        io.write(outbuf.to_slice[0, outbuf.size - outbytesleft])
      end
    end
  end
end

[jgaskins]

@Blacksmoke16 We may want to adjust the label. This issue is not limited to files. The bug occurs with all IO objects when the following criteria are met:

• io.@encoder.@encoding_options.invalid == :skip (I think that's the correct traversal path)
• the IO is writing a string to the IO (specifically a String — not bytes, numbers, etc)
• the string being written is larger than the IO::Encoder's internal buffer, currently hardcoded to 1024B

[Blacksmoke16]

We don't currently have an IO specific label, based on https://github.com/crystal-lang/crystal/commits/master/src/io.cr it seems the convention in past PRs was to use topic:stdlib:files so that's what I went with.

[jgaskins]

Ah, okay. Just wanted to make sure it wasn't just based on the mention of File in the issue title. 😄

[ysbaddaden]

Changed for text. This is an issue with the iconv integration, not IO.