[
https://issues.apache.org/jira/browse/THRIFT-6279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18116344#comment-18116344
]
Jens Geyer commented on THRIFT-6279:
------------------------------------
PR #3877 implements this. Two things turned out differently from the
description:
* Reading failed in more places than the integer conversion. The parser hands
over keys and strings as binaries, so field ids ({{list_to_integer/1}}) and
type names ({{json_to_typeid/1}}) failed too. Four more things broke reading:
** Container type names were never turned into the type ids {{thrift_protocol}}
checks.
** Doubles went through {{list_to_float/1}}, although the parser already
returns floats.
** The stop field took the struct's {{end_object}} with it.
** The message name came back as a binary, where the processor expects a list.
* Larger pieces are possible only up to a point.
{{thrift_socket_transport:read/2}}, and the buffered transport over it, wait
until they have as many bytes as they were asked for. So the protocol asks for
no more bytes than the message still needs at least: one for each open bracket,
plus the end of an open string. A string argument of a call is now read four
bytes per transport call instead of one. Measured: 25,016 calls for a
100,000-byte string.
JSON is not added to the Erlang entry of {{test/tests.json}} yet. The Erlang
JSON protocol still writes booleans and binary fields differently from the
other bindings, which is THRIFT-6289.
_Drafted with AI assistance (Claude Opus 5); reviewed and posted by Jens Geyer._
> Erlang: the JSON protocol cannot read a message and writes binary strings
> unquoted
> ----------------------------------------------------------------------------------
>
> Key: THRIFT-6279
> URL: https://issues.apache.org/jira/browse/THRIFT-6279
> Project: Thrift
> Issue Type: Bug
> Components: Erlang - Library
> Reporter: Jens Geyer
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Found while working on THRIFT-6269. {{thrift_json_protocol}} does not
> round-trip a message. The Erlang cross test covers only binary and compact,
> and {{lib/erl/test}} has no JSON test, so nothing notices. Measured on master
> with OTP 25 and 28.
> h2. 1. Reading any message fails
> {{convert_data(integer, I)}} calls {{list_to_integer(I)}}. That dates from
> THRIFT-1227, when the protocol used jsx. THRIFT-1357 (2014) replaced jsx with
> {{thrift_json_parser}}, whose {{format_number/1}} already returns integers.
> Since then {{list_to_integer/1}} is handed an integer.
> * A message the protocol wrote itself fails to read at its version number
> with {{badarg}}.
> * The integer clause of {{read(_, byte)}} has the same call.
> h2. 2. A string given as a binary is written without quotes
> The generator's default {{string=both}} accepts a string field as a binary,
> but {{write(_, \{string, Str\})}} writes a binary unchanged.
> * Measured: the field comes out as {{"str":abc}}, which is not JSON.
> * A string given as a list is quoted, but nothing in it is escaped.
> h2. 3. Over a socket the read never ends
> {{read_all/1}} reads until the transport returns nothing more, which a socket
> or framed transport never does: it waits for more data.
> Measured with {{thrift_socket_server}} and {{thrift_client_util}} using the
> JSON protocol, both buffered and framed:
> * The server process dies at its 500 ms receive timeout with a
> {{case_clause}} on {{\{error,timeout\}}}.
> * The client call returns an error with {{case_clause}} on
> {{\{error,closed\}}}.
> * The binary protocol works in the same setup.
> h2. 4. One transport call per byte
> {{read_all_1/2}} asks the transport for one byte at a time. This is the
> second point of THRIFT-6269, left out of PR #3867 because of 1 to 3.
> h2. Suggested
> * Fix 1 and 2, with round-trip tests over a memory buffer. For the list case,
> the test should include characters that need escaping.
> * Make the read end with the message, not with the stream: parse
> incrementally, or read what the parser still needs.
> * Then read in larger pieces, and resolve the {{\{error, 'EOF'\}}} clause in
> {{read_all_1/2}}. Its only producer, {{thrift_http_transport:read/2}}, cannot
> reach it.
> * Add JSON to the Erlang entry of {{test/tests.json}} once it works.
> _Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens
> Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
