sile / jsone

Erlang JSON library
MIT License
291 stars 72 forks source link

Handling of ASCII control characters in strings (encode) #10

Closed bkolodziej closed 7 years ago

bkolodziej commented 8 years ago

Hi,

I am using jsone to output JSON in API consumed by various clients. One of issues I had was that following output:

jsone:encode(<<"\x01">>). <<34,1,34>>

caused issues for python clients (unless they used json.loads(data, strict=False)).

I have added following lines:

escape_string(<<0:4, C:4, Str/binary>>, Nexts, Buf, Opt) -> escape_string(Str, Nexts, <<Buf/binary, $\\, $u, $0, $0, ?H8(C)>>, Opt); escape_string(<<1:4, C:4, Str/binary>>, Nexts, Buf, Opt) -> escape_string(Str, Nexts, <<Buf/binary, $\\, $u, $0, $0, ?H8(C + 16)>>, Opt);

before

escape_string(<<0:1, C:7, Str/binary>>, Nexts, Buf, Opt) -> escape_string(Str, Nexts, <<Buf/binary, C>>, Opt);

which causes the output for control characters to change to something that can be consumed by json.loads(data) without disabling strict mode:

jsone:encode(<<"\x01">>). <<"\"\\u0001\"">>

On a side note, I have also added:

value({struct,Value}, Nexts, Buf, Opt) -> object(Value, Nexts, Buf, Opt);

to jsone_encode for easier migration from mochijson2.

Please consider adding above lines to jsone.

sile commented 8 years ago

ASCII control characters

Thank you for pointing it out.

I modified a clause of escape_string function as follows at the commit:

escape_string(<<0:1, C:7, Str/binary>>, Nexts, Buf, Opt) ->
    %% NOTE: This code was a bit faster than above one in my environment
    case C < 16#20 of
        true  -> escape_string(Str, Nexts, <<Buf/binary, "\\u00", ?H8(C)>>, Opt);
        false -> escape_string(Str, Nexts, <<Buf/binary, C>>, Opt)
    end;

On a side note, I have also added: value({struct,Value}, Nexts, Buf, Opt) -> object(Value, Nexts, Buf, Opt); to jsone_encode for easier migration from mochijson2.

This is a valuable feedback. But, I want to make jsone as simple as possible (It already has three object formats for historically reasons ...). So, it would be helpful if conversions for a migration cloud be handled on a upper layer.

sile commented 7 years ago

Since a long period of time elapsed without no reaction, I will close this issue. If you still have any problems, please create a new issue.