[erlang-patches] Add specific return for incomplete asn.1 packets

[Kenneth Lundin]

Hi Jeremy,

I have looked at your patch and we will not include it as it is now.

My comments:

The task to check whether a data packet in this case encoded as PER or BER
is complete should be done on a higher level instead of
cluttering all the code with these checks. I think that this is already
done in most implementations of protocols. What else would you do? Take a
chance and
try to decode and try again if you get error, incomplete? I don't think so.

I think it is always possible to check that you have got the whole PDU
before starting to decode the data. Then the decoding routines can just
assume
that you have all the data.

I am not even sure that it is possible to determine in all places if a
badmatch is because of incomplete data or faulty data.

Another note is that the module asn1rt_ber_bin will soon be obsoleted and
asn1rt_ber_bin_v2 will be the default. In that module it is possible to do
a generic check
for incomplete, and there are also NIF's involved which can get out of data
and should the return incomplete.
But you could not know that asn1rt_ber_bin will be obsoleted soon.

Even if I am not convinced of the need of this check I might accept it if
it has no negative impact on performance and does not increase the size of
runtime modules and generated code significantly, and if it really is safe
to tell that it failed because of incomplete data.

/Regards Kenneth, Erlang/OTP Ericsson

On Tue, Jul 24, 2012 at 5:15 PM, Jeremy Heater <
jeremy.heater@REDACTED> wrote:

> Hello all,
>
> We had already broached the subject a few years back (cf.
> http://erlang.org/pipermail/erlang-patches/2009-December/000627.html) and
> we modified our approach to take the comments made at the time into
> account. Here follows a description of the changes:
>
> This patch's goal is to allow the user to determine if an incoming
> asn.1 packet is incomplete.
> To that end, the decode/2 functions of generated asn.1 files were
> modified so as to return {error, incomplete} when the packet to be
> decoded is incomplete instead of the generic
> {error, {asn1, {badmatch, _}}} (or badarg) which is not distinguishable
> from other errors.
>
> This would seem to us to be a necessary feature so as to be able to
> determine if a packet received from a socket is complete (since overly
> large messages are split).
> It is also complementary to the undec_rest option where when decode/2
> is called on a packet containing more than one message, the function
> returns the first message decoded and the rest of the packet
> undecoded.
>
> The solution was implemented for all encoding rules, even though BER
> encoding allows for simpler checking by manually parsing the first
> bytes of the packet, for the sake of having a consistent API whatever
> the encoding.
>
> The proposed patch can be found at:
> git fetch git://github.com/Kwisatx/otp.git incomplete_packet_asn1
>
> https://github.com/Kwisatx/otp/compare/master...incomplete_packet_asn1
>
> https://github.com/Kwisatx/otp/compare/master...incomplete_packet_asn1.patch
>
>
> Since the added tests inevitably add some overhead, we also tried to
> link the changes to the undec_rest option
> (incomplete_packet_asn1_duplicate_files branch), but since most of the
> changes are in the runtime, it made duplicating the code necessary (as
> the name of the branch suggests), which complicates the code.
>
> What are your thoughts on this change? Any ideas for how to improve it?
>
> Best regards,
>
> Jeremy Heater.
> _______________________________________________
> erlang-patches mailing list
> erlang-patches@REDACTED
> http://erlang.org/mailman/listinfo/erlang-patches
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20120823/1e222ef8/attachment.htm>