diff mbox series

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

Message ID 20241028105326.3159618-1-quic_amisjain@quicinc.com (mailing list archive)
State Superseded
Headers show
Series [v1] obex : Fix for PBAP GET request in PTS testing | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck success Bluez Make Check PASS
tedd_an/MakeDistcheck success Make Distcheck PASS
tedd_an/CheckValgrind success Check Valgrind PASS
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/IncrementalBuild success Incremental Build PASS
tedd_an/ScanBuild success Scan Build PASS

Commit Message

Amisha Jain Oct. 28, 2024, 10:53 a.m. UTC
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(+)

Comments

bluez.test.bot@gmail.com Oct. 28, 2024, 12:39 p.m. UTC | #1
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
Luiz Augusto von Dentz Oct. 28, 2024, 2:26 p.m. UTC | #2
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
>
>
Amisha Jain Oct. 29, 2024, 11:58 a.m. UTC | #3
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
Luiz Augusto von Dentz Oct. 29, 2024, 2:14 p.m. UTC | #4
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
diff mbox series

Patch

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);