| Message ID | 20241028105326.3159618-1-quic_amisjain@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] obex : Fix for PBAP GET request in PTS testing | expand |
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
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 > >
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
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 --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);