Re: [PATCH v4] obex : Fix for PBAP GET request in PTS testing

[Paul Menzel <pmenzel@xxxxxxxxxxxxx>]

Dear Amisha,


Thank you for your patch. For the commit message summary, it’d be great 
if you made it a statement by using a verb in imperative mood, and 
removed the space before the colon:

obex: Fix PBAP GET request in PTS testing

Am 01.11.24 um 10:33 schrieb Amisha Jain:
> This change is required for passing below PTS testcases -

I’d use a colon instead of a hyphen.

> 1. PBAP/PSE/PBD/BV-02-C
> 2. PBAP/PSE/PBD/BV-03-C
> 3. PBAP/PSE/PBD/BI-01-C
> 4. PBAP/PSE/PBD/BV-13-C
> 5. PBAP/PSE/PBD/BV-14-C
> 6. PBAP/PSE/PBD/BV-17-C
>
> For all the GET phonebook request sent by PTS has no extra params
> added in it, therefore PBAP server is rejecting the request

Maybe: PTS sends all the GET phonebook requests without extra params. 
Therefore, the PBAP server is rejecting the requests with a `Bad 
Request` response.

> by sending 'Bad Request' as response.
> So appending few default params in GET request to avoid

s/appending/append/

> testcase failure.
> This params are already added for Vcardlisting and Vcardentry

s/This/These/

> operations.
>
> ---
>   obexd/plugins/pbap.c | 23 ++++++++++++++++++++++-
>   1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/obexd/plugins/pbap.c b/obexd/plugins/pbap.c
> index 4175f9de8..f70a17cdf 100644
> --- a/obexd/plugins/pbap.c
> +++ b/obexd/plugins/pbap.c
> @@ -511,7 +511,23 @@ static int pbap_get(struct obex_session *os, void *user_data)
>   		rsize = 0;
>   	}
>   
> -	/* Workaround for PTS client not sending mandatory apparams */
> +	/*
> +	 * Workaround for PTS client not sending mandatory apparams
> +	 *
> +	 * Add MaxListCount attribute, description as per PBAP spec
> +	 *
> +	 * 5.1.4.3 MaxListCount :
> +	 * This header is used to indicate the maximum number of
> +	 * entries of the <x-bt/phonebook> object that the PCE
> +	 * can handle. The value 65535 means that the number of
> +	 * entries is not restricted. The maximum number of entries
> +	 * shall be 65,535 if this header is not specified.
> +	 *
> +	 * 0x04 - Tag id
> +	 * 0x02 - length
> +	 * next 2 bytes are value - 0xffff
> +	 */
> +
>   	if (!rsize && g_ascii_strcasecmp(type, VCARDLISTING_TYPE) == 0) {
>   		static const uint8_t default_apparams[] = {
>   			0x04, 0x02, 0xff, 0xff
> @@ -524,6 +540,11 @@ static int pbap_get(struct obex_session *os, void *user_data)
>   		};
>   		buffer = default_apparams;
>   		rsize = sizeof(default_apparams);
> +	} else if (!rsize && g_ascii_strcasecmp(type, PHONEBOOK_TYPE) == 0) {
> +		static const uint8_t default_apparams[] = {
> +			0x04, 0x02, 0xff, 0xff };

Above in the existing code, the curly brace seems to go on a separate line.

> +		buffer = default_apparams;
> +		rsize = sizeof(default_apparams);
>   	}
>   
>   	params = parse_aparam(buffer, rsize);