Fix unicode bug with "format_raw_parse_error" sample

[mattbaker]

Bit of a strange one, but I think I tracked it down, so I'm curious what folks think.

We ran into this issue after a user started sending some strange input our way that was (I suspect correctly) failing to parse. The weird part was we were seeing exceptions inside Absinthe.Plug as it attempted to JSON encode an error message to send back to the user, the exceptions looked something like this:

(Jason.EncodeError) invalid byte 0xE2 in <<80, 97, 114, 115, 105, 110, 103, 32, 102, 97, 105, 108, 101, 100, 32, 97, 116, 32, 96, 226, 128, 156, 110, 97, 109, 101, 61, 226, 128, 96>>

(jason 1.2.2) lib/jason.ex:150: Jason.encode!/2
(absinthe_plug 1.5.0) lib/absinthe/plug.ex:507: Absinthe.Plug.encode/4
(endpoint 0.0.1) lib/plug/router.ex:284: Endpoint.Pipeline.dispatch/2
[...]

The bytes 226 (0xE2) and 128 (0x80) are of interest here. If you remove them we have a valid string again:

"Parsing failed at `“name=`"

Which leads us to Absinthe.Phase.Parse.format_raw_parse_error/1 where that message is generated.

  defp format_raw_parse_error({:lexer, rest, {line, column}}) do
    <<sample::binary-size(10), _::binary>> = rest

    message = "Parsing failed at `#{sample}`"
    %Phase.Error{message: message, locations: [%{line: line, column: column}], phase: __MODULE__}
  end

My hypothesis is this:

• Bad input arrived and failed to parse
• <<sample::binary-size(10), _::binary>> grabbed the problematic character and the next ten bytes for the sample.
• The tenth character happened to be a unicode character, represented by multiple bytes. For example, an em-dash (— == <<226, 128, 148>>).
• The binary pattern match truncated the string in the middle of the UTF8 character, producing a binary that was no longer a valid string (a string ending in <<226, 128>> for example)

In practice no exception occurred until the output of Absinthe.Phase.Parse.format_raw_parse_error/1 made it to the JSON encoding step in the Absinthe.Plug pipeline, which was expecting a valid string. I think the tests here can still demonstrate the edge case though.

Without knowing more, the most straightforward solution I can think of is to use String.slice which is unicode aware unless there's a reason we can't use it, but it did get the tests passing.

The tests demonstrate two things. The first is a repro of the error I believe we're experiencing, the second is an edge case with very short, invalid documents that occurred to me while I was looking at the code.

[mattbaker]

If we're thinking character-oriented and not byte oriented I would expect 10 characters. In practice this test will fail because the message will include a sample of ten bytes.

[mattbaker]

Not something we experienced, but something that occurred to me when I was thinking about the binary pattern match.

[binaryseed]

Great find! Seems like your proposed fix is a good one, let's see those tests turn green

[mattbaker]

Awesome, updated with the fix @binaryseed !