Wireshark · Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] rev 32519: /trunk/epan/dissectors/ /trunk/epan/dissectors/:

[Jaap Keuter <jaap.keuter@xxxxxxxxx>]

Hi,

Why the strong position that "it must be a dissector bug"?
From a comment? What if that comment was misguided, and the author just didn't 
know how to handle these cases/wasn't aware they exist? Even the original text 
added to the tree says "Unknown".
If this really is a problem then the dissector should be fixed, or rolled back.
Running randpkt into the ground like this isn't the right way forward.

I vote for rolling back rev 32519[1] and not backport it to the stable branch.

[1] 
http://anonsvn.wireshark.org/viewvc/trunk/epan/dissectors/packet-rsvp.c?r1=32519&r2=32518&pathrev=32519

Thanks,
Jaap

On 04/28/2010 10:51 PM, Guy Harris wrote:
> On Apr 28, 2010, at 1:32 PM, Jeff Morriss wrote:
>
> > guy@xxxxxxxxxxxxx wrote:
> > > http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=32519
> > >
> > > User: guy
> > > Date: 2010/04/19 04:38 PM
> > >
> > > Log:
> > > If that should truly "never happen", use DISSECTOR_ASSERT_NOT_REACHED()
> > > so it's more clearly marked as a dissector bug.
> > >
> > > (It apparently *does* happen - see bug 4698.)
> >
> > This has the randpkt test failing on the buildbot.
>
> ...which means that the RSVP dissector has, and had even before that checkin, a bug, in that something that, according to a comment in the code, "should never happen" can, in fact, happen with a bogus packet; this just makes the bug more obvious.
>
> > Should it really be backported to 1.2.8?
>
> Clearly marking something that "should never happen" but does happen as a dissector bug in the dissection is better than just putting a blob of
>
> 	Unknown session type
>
> into the protocol tree, so, yes, I'd backport it.
>
> > Or should the randpkt test accept dissector bugs as OK (like the fuzz
> > testing)?
>
> The fuzz testing accepts dissector bug reports as OK?  That seems like an error to me.