1212729 - Bluetooth PBAP doesn’t handle illegal virtual folders path properly.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Jamin Liu [:jaliu][:Jamin]]

PBAP manager should reply ObexResponseCode::NotFound if the path virtual folders path is illegal.

Comment 1

[Jamin Liu [:jaliu][:Jamin]]

Attachment: Handle illegal PBAP virtual folders path properly. (v1)

Comment 2

[Ben Tian (inactive)]

Comment on attachment 8671181 [details] [diff] [review]
Handle illegal PBAP virtual folders path properly. (v1)

Review of attachment 8671181 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comment addressed.

::: dom/bluetooth/bluedroid/BluetoothPbapManager.cpp
@@ +403,5 @@
>        // Go back to root
>        newPath.AssignLiteral("");
>      } else {
>        // Go down 1 level
> +      if (!mCurrentPath.IsEmpty()) {

Replace |mCurrentPath| with |newPath|.

::: dom/bluetooth/bluedroid/BluetoothPbapManager.h
@@ +157,5 @@
>    InfallibleTArray<uint32_t>  PackPropertiesMask(uint8_t* aData, int aSize);
>    bool CompareHeaderTarget(const ObexHeaderSet& aHeader);
>    bool IsLegalPath(const nsAString& aPath);
> +  bool IsLegalPhonebookName(const nsAString& aName);
> +  bool GetInputStreamFromBlob(nsIDOMBlob* aBlob);

Restore |nsIDOMBlob| to |Blob|.

Comment 3

[Ben Tian (inactive)]

Comment on attachment 8671181 [details] [diff] [review]
Handle illegal PBAP virtual folders path properly. (v1)

Review of attachment 8671181 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/bluedroid/BluetoothPbapManager.cpp
@@ +627,5 @@
> +    "SIM1/telecom/pb",
> +    "SIM1/telecom/ich",
> +    "SIM1/telecom/och",
> +    "SIM1/telecom/mch",
> +    "SIM1/telecom/cch"

To note, the reason to remove "/" is to keep internal phonebook path consistent to path carried in PullPhoneBook request (i.e., no "/" at the beginning), per PBAP spec 1.2.

5.1.2 Name 
The Name header shall contain the absolute path in the virtual folders architecture of the PSE, appended with the name of the file representation of one of the Phone Book Objects. 
Example: telecom/pb.vcf or SIM1/telecom/pb.vcf for the main phone book objects.

Comment 4

[Ben Tian (inactive)]

Set 2.2r blocker for required bluetooth feature PBAP.

Comment 5

[Jamin Liu [:jaliu][:Jamin]]

Attachment: (for m-c) Handle illegal PBAP virtual folders path properly. (v2), r=btian

- revise previous patch based on #comment 2.

Thank Ben for reviewing the patch.

Comment 6

[Jamin Liu [:jaliu][:Jamin]]

The results on treeherder looks fine.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1d44e588a86

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/83f9e85ebee0

Comment 8

[Carsten Book [:Tomcat]]

this failed to apply to 2.2r with:

patching file dom/bluetooth/bluedroid/BluetoothPbapManager.h
Hunk #1 FAILED at 151
1 out of 1 hunks FAILED -- saving rejects to file dom/bluetooth/bluedroid/BluetoothPbapManager.h.rej

could you take a look, thanks!

Comment 9

[Jamin Liu [:jaliu][:Jamin]]

(In reply to Carsten Book [:Tomcat] from comment #8)
> this failed to apply to 2.2r with:
> 
> patching file dom/bluetooth/bluedroid/BluetoothPbapManager.h
> Hunk #1 FAILED at 151
> 1 out of 1 hunks FAILED -- saving rejects to file
> dom/bluetooth/bluedroid/BluetoothPbapManager.h.rej
> 
> could you take a look, thanks!

Got it.
I'll upload a patch for v2.2r.
Sorry for the inconvenience caused.

Comment 10

[Jamin Liu [:jaliu][:Jamin]]

Attachment: (for 2.2r) Handle illegal PBAP virtual folders path properly. (v2), r=btian, a=2.2r

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/83f9e85ebee0

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/229aa56d01b1