Fwd: [erlang-patches] Re: [Re] Patch for asn1 library

[Vincent de Phily]

On Monday 21 December 2009 17:12:26 aude quintana wrote:
> ---------- Forwarded message ----------
> From: Kenneth Lundin <kenneth.lundin@REDACTED>
> Date: 2009/12/21
> Subject: Re: [erlang-patches] Re: [Re] Patch for asn1 library
> To: Aude Quintana <aude.quintana@REDACTED>
> Cc : erlang-patches@REDACTED
> 
> On Thu, Dec 17, 2009 at 10:16 AM, Aude Quintana
> 
> <aude.quintana@REDACTED> wrote:
> > To remind you the subject it is a patch which change decode/2
> > functions generated from a file .asn1 : they return the error {error,
> > incomplete} when the packet given to decode is incomplete instead of
> > any error.
> > It is necessary when implementing functions that decode packet
> > received from a socket when it's not sure that they are complete (if
> > messages are too big, they are are splitted).
> 
> I don't really agree with you here.
> When receiving data on a socket you can never know for sure you get all
>  data together in one message (or revc) or if you have to collect and
> concatenate data from many messages
> until you have a complete PDU. In most protocols you have some header
> with a length indicator which
> tell you how many bytes you need to get a complete package i.e you
> must handle this before you
> call asn1rt:decode.
> When you open a socket you have the packet mode which can help with
> this, there is also a specific
> asn.1 packet mode there.

Yes, we are aware of this solution (either implemented manually or using 
inet:setopts/2) but it adds a packet overhead that is not fundamantally 
necessary. Trading 4 bytes on the network against the CPU cost of potentially 
re-parsing a whole asn1 packet is a compromise. In our case we pay per byte 
sent, so we save every one we can.

> When the decoder fails the rason can be any of:
> 1) corrupt indata , for example incorrect length indication
> 2) incomplete data i.e. the case you are after with missing data
> 3) a bug in the Erlang asn1 decoding functions

1) and 3) happen exceptionally, we have little choice but to close the 
connection. 2) happens normaly, we just wait for more data.

> You can not know which case of the above in runtime.

That's what our patch is for : it enables us to tell the difference.

> Another point is that even if you return {error,incomplete} , what can
> the user do about that?
> It is not a good idea to rely on that for regular use for testing if
> more data is needed simply because it is not efficient,
> you have fetch more data and then do the full decode again.
> The right thing is as said above to first assure that a complete PDU
> is at hand and then call decode.

I understand your point, but as I said it is a bytes / CPU tradeoff and we are 
willing to save the former and waste the later.

> Yes i agree here that "incomplete2 is kind of a complement to
> {Result,RestBytes} but
> with the big difference that it makes much more sense to continue
> computing something with the RestBytes
> and that it makes sense that you get RestBytes for legitime reasons
> and not becauseof an unknown type of error.

Then again, you are suggesting to use an extra non-asn1 header to know the 
size of the asn1 packet. If we know the size of the packet, then having the 
RestBytes functionality is superfluous : split_binary/2 works well enough.

To me, having a RestBytes functionality seems of little use without a 
corresponding MissingBytes functionality : either we rely on the asn1 library 
to do packet spliting for us (but the library needs to implement it fully), or 
we do packet spliting ourselves (and the library functionality is useless).


> Above was a discussion regarding the need and usefulness of
>  {error,incomplete}.
> 
> I also think that the tests you have added will cause a significant
> degradation of performance for all users
> even those that are not interested in {error,incomplete}.
> If {error,incomplete} should be supported as return value (which I
> don't think is a very good idea as said above)
> I would implement it in another way with less performance penalty and
> for example make it an option at
> compile time for those who want the function.

If it is possible to implement {error,incomplete} in a more efficient way, 
we'd be happy to. Give us some pointers to improve things, unless you'd like 
to refactor our patch yourself.

Concerning the compile-time option, it is probably a good idea, as all these 
extra check will never be overhead-free. Maybe use the existing undec_rest 
compile option for both {error,incomplete} and RestBytes (as explained above, 
I dont think one is usefull without the other) ?


> There is also a big difference between BER and PER in this regard. In
> BER the nested TLV structure is
> decoded first and sometimes even in a driver (C-code) and the checking
> can be done there (only).
> In PER the length information is ofthen missing and the checking must
> be done by other means.

Indeed. In a previous version of our protocol we used BER, and manually parsed 
the first few bytes to know if the asn1 packet was complete or not (and wether 
there was extra data afterwards). But our current protocol uses UPER, and 
reading the first few bytes to know the size is no longer possible. We either 
need an extra header giving us the packet size, or some help from the asn1 
library.
-- 
Vincent de Phily
Mobile Devices
+33 (0) 142 119 325
+353 (0) 85 710 6320 

Warning
This message (and any associated files) is intended only for the use of its
intended recipient and may contain information that is confidential, subject
to copyright or constitutes a trade secret. If you are not the intended
recipient you are hereby notified that any dissemination, copying or
distribution of this message, or files associated with this message, is
strictly prohibited. If you have received this message in error, please
notify us immediately by replying to the message and deleting it from your
computer. Any views or opinions presented are solely those of the author
vincent.dephily@REDACTED and do not necessarily represent those of 
the
company. Although the company has taken reasonable precautions to ensure no
viruses are present in this email, the company cannot accept responsibility
for any loss or damage arising from the use of this email or attachments.