IO::Hexdump for binary protocol inspection #3043

ysbaddaden · 2016-07-25T19:57:43Z

Rework Slice#Hexdump to separate each byte, instead of each short, and add an initial hex index (more common representation of hexdumps).
Add IO::Hexdump wrapping IO that prints a hexdump when reading or writing bytes from any IO.

Example:

00000000  20 21 22 23 24 25 26 27  28 29 2a 2b 2c 2d 2e 2f   !"#$%&'()*+,-./
00000010  30 31 32 33 34 35 36 37  38 39 3a 3b 3c 3d 3e 3f  0123456789:;<=>?
00000020  40 41 42 43 44 45 46 47  48 49 4a 4b 4c 4d 4e 4f  @ABCDEFGHIJKLMNO
00000030  50 51 52 53 54 55 56 57  58 59 5a 5b 5c 5d 5e 5f  PQRSTUVWXYZ[\\]^_
00000040  60 61 62 63 64 65 66 67  68 69 6a 6b 6c 6d 6e 6f  `abcdefghijklmno
00000050  70 71 72 73 74 75 76 77  78 79 7a 7b 7c 7d 7e 7f  pqrstuvwxyz{|}~.
00000060  80 81 82 83 84                                    .....

bcardiff · 2016-07-25T20:09:18Z

src/io/hexdump.cr

+  class Hexdump
+    include IO
+
+    def initialize(@io : IO, @output : IO = STDERR, @read = false, @write = false)


shouldn't the default output be STDOUT?

I think it's OK to use STDERR. For example today I was coding something and did ruby app.rb > data.json and wanted to debug it at the same time. For debugging I used STDERR.puts ... so I would get the debug data in the terminal but I would still get the output data in data.json without mixing it with debug code.

Yes, I defaulted to STDERR for this very reason.

ysbaddaden · 2016-08-01T21:46:07Z

@asterite I get a weird error on CI:

in ./src/io/hexdump.cr:38: instantiating '(HTTP::ChunkedContent | HTTP::Server::Response::Output | HTTP::Server::ReverseResponseOutput | HTTP::UnknownLengthContent | HTTP::WebSocket::Protocol::StreamIO | IO::ARGF | IO::BufferedWrapper | IO::Delimited | IO::FileDescriptor | IO::Hexdump | IO::RaiseErrno | IO::Sized | Int32 | JSON::PrettyWriter | MemoryIO | OpenSSL::SSL::Socket | PartialReaderIO | PointerIO | SimpleMemoryIO | String::Builder | Zlib::Deflate | Zlib::Inflate | Nil)#tap()'
      @io.write(buf).tap do |written_bytes|
                     ^~~
in ./src/io/hexdump.cr:39: instantiating 'Slice(UInt8)#[](Int32, (HTTP::ChunkedContent | Int32))'
        @output.puts buf[0, written_bytes].hexdump if @write && written_bytes
                        ^
in ./src/slice.cr:175: no overload matches 'Int32#<=' with type (HTTP::ChunkedContent | Int32)

I modified my patch to output the type of written_bytes as returned by @io.write in IO::Hexdump then run bin/crystal spec spec/std/http/client* spec/std/io/hexdump_spec.cr and it prints the following union instead of (Int32 | Nil):

(HTTP::ChunkedContent | HTTP::Server::Response | HTTP::Server::Response::Output | HTTP::UnknownLengthContent | HTTP::WebSocket::Protocol::StreamIO | IO::ARGF | IO::Delimited | IO::FileDescriptor | IO::Hexdump | IO::Sized | Int32 | MemoryIO | OpenSSL::SSL::Socket | PointerIO | String::Builder | Zlib::Deflate | Zlib::Inflate | Nil)

asterite · 2016-08-02T14:15:49Z

@ysbaddaden Oh!! I just remembered: IO#write(slice) writes the whole slice, and the return value doesn't matter. So in write's case we can hexdump the whole slice. And in read's case there's no need for the cast.

ysbaddaden · 2016-08-02T14:51:07Z

OK I'll change that.

There is still something odd. For example all HTTP::ChunkedContent#write does is raising an exception, hence it's a NoReturn, but the compiler considers HTTP::ChunkedContent to be a possible return type for IO#write.

- initial 32bits hex index - separate each byte (instead of each short) - extra separator every 8 bytes

Transparent IO that prints an hexadecimal dump of read and written bytes. Mostly useful for inspecting and debugging binary protocols.

ysbaddaden · 2016-08-02T14:54:42Z

done.

asterite · 2016-08-02T15:00:47Z

@ysbaddaden I'll investigate it. Maybe some other IO returns self or something like that.

asterite · 2016-08-02T15:12:39Z

@ysbaddaden Found it, it's this line which got the type from this line

I think eventually it would be nice if a parent type could set the type for child methods, so we could mark IO#write as returning Nil, and so all implementors would return Nil too.

I also have to check why the IO is expanded there to all the types...

asterite · 2016-08-02T15:39:50Z

@ysbaddaden Thank you!! :-)

bcardiff reviewed Jul 25, 2016
View reviewed changes

ysbaddaden force-pushed the std-io-hexdump branch from b618685 to bc7b95e Compare July 25, 2016 20:40

jhass added the topic:stdlib label Aug 2, 2016

ysbaddaden added 2 commits August 2, 2016 16:53

ysbaddaden force-pushed the std-io-hexdump branch from 55c9445 to ce0ffc3 Compare August 2, 2016 14:54

asterite merged commit a80db64 into crystal-lang:master Aug 2, 2016

asterite added this to the 0.19.0 milestone Aug 2, 2016

ysbaddaden deleted the std-io-hexdump branch August 2, 2016 20:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IO::Hexdump for binary protocol inspection #3043

IO::Hexdump for binary protocol inspection #3043

ysbaddaden commented Jul 25, 2016 •

edited

Loading

bcardiff Jul 25, 2016

asterite Jul 25, 2016

bcardiff Jul 25, 2016

ysbaddaden Jul 25, 2016

ysbaddaden commented Aug 1, 2016

asterite commented Aug 2, 2016

ysbaddaden commented Aug 2, 2016

ysbaddaden commented Aug 2, 2016

asterite commented Aug 2, 2016

asterite commented Aug 2, 2016

asterite commented Aug 2, 2016

IO::Hexdump for binary protocol inspection #3043

IO::Hexdump for binary protocol inspection #3043

Conversation

ysbaddaden commented Jul 25, 2016 • edited Loading

bcardiff Jul 25, 2016

Choose a reason for hiding this comment

asterite Jul 25, 2016

Choose a reason for hiding this comment

bcardiff Jul 25, 2016

Choose a reason for hiding this comment

ysbaddaden Jul 25, 2016

Choose a reason for hiding this comment

ysbaddaden commented Aug 1, 2016

asterite commented Aug 2, 2016

ysbaddaden commented Aug 2, 2016

ysbaddaden commented Aug 2, 2016

asterite commented Aug 2, 2016

asterite commented Aug 2, 2016

asterite commented Aug 2, 2016

ysbaddaden commented Jul 25, 2016 •

edited

Loading