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

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

[Amisha Jain]

This change is required for passing below PTS testcases -
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
by sending 'Bad Request' as response.
So appending few default params in GET request to avoid
testcase failure.

---
 obexd/plugins/pbap.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/obexd/plugins/pbap.c b/obexd/plugins/pbap.c
index 4175f9de8..3c23815ba 100644
--- a/obexd/plugins/pbap.c
+++ b/obexd/plugins/pbap.c
@@ -524,6 +524,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 };
+		buffer = default_apparams;
+		rsize = sizeof(default_apparams);
 	}
 
 	params = parse_aparam(buffer, rsize);
-- 
2.34.1

RE: [v1] obex : Fix for PBAP GET request in PTS testing

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 949 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=903757

---Test result---

Test Summary:
CheckPatch                    PASS      0.46 seconds
GitLint                       PASS      0.32 seconds
BuildEll                      PASS      24.52 seconds
BluezMake                     PASS      1676.90 seconds
MakeCheck                     PASS      13.61 seconds
MakeDistcheck                 PASS      179.65 seconds
CheckValgrind                 PASS      252.62 seconds
CheckSmatch                   PASS      356.45 seconds
bluezmakeextell               PASS      120.25 seconds
IncrementalBuild              PASS      1428.82 seconds
ScanBuild                     PASS      1002.59 seconds



---
Regards,
Linux Bluetooth

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

[Luiz Augusto von Dentz]

Hi Amisha,

On Mon, Oct 28, 2024 at 7:10 AM Amisha Jain <quic_amisjain@quicinc.com> wrote:
>
> This change is required for passing below PTS testcases -
> 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
> by sending 'Bad Request' as response.
> So appending few default params in GET request to avoid
> testcase failure.
>
> ---
>  obexd/plugins/pbap.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/obexd/plugins/pbap.c b/obexd/plugins/pbap.c
> index 4175f9de8..3c23815ba 100644
> --- a/obexd/plugins/pbap.c
> +++ b/obexd/plugins/pbap.c
> @@ -524,6 +524,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) {

Hmm, where do these values come from though? Are they defined in the
PBAP spec, if they are then we can probably quote the spec.

> +               static const uint8_t default_apparams[] = {
> +                       0x04, 0x02, 0xff, 0xff };
> +               buffer = default_apparams;
> +               rsize = sizeof(default_apparams);
>         }
>
>         params = parse_aparam(buffer, rsize);
> --
> 2.34.1
>
>


-- 
Luiz Augusto von Dentz

RE: [PATCH v1] obex : Fix for PBAP GET request in PTS testing

[Amisha Jain (QUIC)]

Hi Luiz,
The default param added here is corresponding to 'MaxlistCount' attribute (tag id - 0x04) which should be 65535 if it is not specified by client as per the PBAP spec. The value 65535 means that the number of entries is not restricted.
Here is the explanation of application parameter header - 
default_apparams[] = { 0x04, 0x02, 0xff, 0xff }

      0x04 - Tag id (for MaxlistCount)
      0x02 - length
      next 2 bytes are value - 0xffff

Thanks,
Amisha

> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Monday, October 28, 2024 7:57 PM
> To: Amisha Jain (QUIC) <quic_amisjain@quicinc.com>
> Cc: linux-bluetooth@vger.kernel.org; Mohammed Sameer Mulla (QUIC)
> <quic_mohamull@quicinc.com>; Harish Bandi (QUIC)
> <quic_hbandi@quicinc.com>; Anubhav Gupta (QUIC)
> <quic_anubhavg@quicinc.com>
> Subject: Re: [PATCH v1] obex : Fix for PBAP GET request in PTS testing
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> Hi Amisha,
> 
> On Mon, Oct 28, 2024 at 7:10 AM Amisha Jain <quic_amisjain@quicinc.com>
> wrote:
> >
> > This change is required for passing below PTS testcases - 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 by sending
> > 'Bad Request' as response.
> > So appending few default params in GET request to avoid testcase
> > failure.
> >
> > ---
> >  obexd/plugins/pbap.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/obexd/plugins/pbap.c b/obexd/plugins/pbap.c index
> > 4175f9de8..3c23815ba 100644
> > --- a/obexd/plugins/pbap.c
> > +++ b/obexd/plugins/pbap.c
> > @@ -524,6 +524,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) {
> 
> Hmm, where do these values come from though? Are they defined in the
> PBAP spec, if they are then we can probably quote the spec.
> 
> > +               static const uint8_t default_apparams[] = {
> > +                       0x04, 0x02, 0xff, 0xff };
> > +               buffer = default_apparams;
> > +               rsize = sizeof(default_apparams);
> >         }
> >
> >         params = parse_aparam(buffer, rsize);
> > --
> > 2.34.1
> >
> >
> 
> 
> --
> Luiz Augusto von Dentz

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

[Luiz Augusto von Dentz]

Hi Amisha,

On Tue, Oct 29, 2024 at 7:59 AM Amisha Jain (QUIC)
<quic_amisjain@quicinc.com> wrote:
>
> Hi Luiz,
> The default param added here is corresponding to 'MaxlistCount' attribute (tag id - 0x04) which should be 65535 if it is not specified by client as per the PBAP spec. The value 65535 means that the number of entries is not restricted.
> Here is the explanation of application parameter header -
> default_apparams[] = { 0x04, 0x02, 0xff, 0xff }
>
>       0x04 - Tag id (for MaxlistCount)
>       0x02 - length
>       next 2 bytes are value - 0xffff

Let's have it as comments of this code, preferably quoting the spec
text directly including the section and page.

> Thanks,
> Amisha
>
> > -----Original Message-----
> > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> > Sent: Monday, October 28, 2024 7:57 PM
> > To: Amisha Jain (QUIC) <quic_amisjain@quicinc.com>
> > Cc: linux-bluetooth@vger.kernel.org; Mohammed Sameer Mulla (QUIC)
> > <quic_mohamull@quicinc.com>; Harish Bandi (QUIC)
> > <quic_hbandi@quicinc.com>; Anubhav Gupta (QUIC)
> > <quic_anubhavg@quicinc.com>
> > Subject: Re: [PATCH v1] obex : Fix for PBAP GET request in PTS testing
> >
> > WARNING: This email originated from outside of Qualcomm. Please be wary
> > of any links or attachments, and do not enable macros.
> >
> > Hi Amisha,
> >
> > On Mon, Oct 28, 2024 at 7:10 AM Amisha Jain <quic_amisjain@quicinc.com>
> > wrote:
> > >
> > > This change is required for passing below PTS testcases - 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 by sending
> > > 'Bad Request' as response.
> > > So appending few default params in GET request to avoid testcase
> > > failure.
> > >
> > > ---
> > >  obexd/plugins/pbap.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/obexd/plugins/pbap.c b/obexd/plugins/pbap.c index
> > > 4175f9de8..3c23815ba 100644
> > > --- a/obexd/plugins/pbap.c
> > > +++ b/obexd/plugins/pbap.c
> > > @@ -524,6 +524,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) {
> >
> > Hmm, where do these values come from though? Are they defined in the
> > PBAP spec, if they are then we can probably quote the spec.
> >
> > > +               static const uint8_t default_apparams[] = {
> > > +                       0x04, 0x02, 0xff, 0xff };
> > > +               buffer = default_apparams;
> > > +               rsize = sizeof(default_apparams);
> > >         }
> > >
> > >         params = parse_aparam(buffer, rsize);
> > > --
> > > 2.34.1
> > >
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz