Re: [PATCH obexd 2/2] gobex: Fix ABORT request not processing

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Hendrik,

On Fri, Jan 13, 2012, Hendrik Sattler wrote:
>>>> 		opcode = obex->rx_last_op;
>>>> 		/* Unexpected response -- fail silently */
>>>>-		if (opcode > 0x1f && opcode < 0xff) {
>>>>+		if (opcode > G_OBEX_OP_ABORT && opcode < 0xff) {
>>>> 			obex->rx_data = 0;
>>>> 			return TRUE;
>>>> 		}
>>>
>>>This one always evaluates to false because 0xff == G_OBEX_OP_ABORT |
>>>FINAL_BIT.
>>
>> The value of obex->rx_last_op is stored with the final bit cleared,
>> so the < 0xff part could also be removed from the test. The value
>> 0x1f has been picked because the IrDA OBEX specification defines 0x10
>> - 0x1f as a user-definable range, so anything between 0x1f and 0x7f
>> isn't actually valid. The correct test would then become:
>>
>>	if (opcode > 0x1f && opcode != G_OBEX_OP_ABORT)
>>
>> Jaganath, with the above change the patch should be ok, but the more
>> surprising thing to me here is that this implies we're missing one or
>> more unit tests for Abort. Could you create a patch to add those too?
>> (if we're really strict those tests should go in before this patch so
>> that it's actually possible to see that the patch makes a
>> difference).
> 
> User-definable does not mean invalid. It only means that these can be
> used for custom commands. These are still bound to the rules of the
> OBEX protocol. I've never seen one using that, though.

Exactly. You might want to re-read what I said and the test I proposed.

> Else you'd also have to filter 0x04 and the range 0x08-0x0f because
> these are marked as "reserved".

With the test I proposed we filter neither reserved (since they might
get meaning in new spec versions) nor user-defined opcodes.

Johan