Bug 198819 - [regression] bluetoothctl commands containing newlines are no longer parsed and executed
Summary: [regression] bluetoothctl commands containing newlines are no longer parsed a...
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Bluetooth (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: linux-bluetooth@vger.kernel.org
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-19 10:15 UTC by Daniel van Vugt
Modified: 2018-02-21 06:59 UTC (History)
1 user (show)

See Also:
Kernel Version: 4.13.0
Subsystem:
Regression: No
Bisected commit-id:


Attachments
fix-198819-v1.patch (1.27 KB, patch)
2018-02-20 05:09 UTC, Daniel van Vugt
Details | Diff
Don't process HUP cond before others (1.21 KB, patch)
2018-02-20 09:18 UTC, Luiz Von Dentz
Details | Diff

Description Daniel van Vugt 2018-02-19 10:15:33 UTC
This problem was encountered in python-dbusmock which tries to pipe multiple commands into bluetoothctl:

    out, err = process.communicate(input='list\n' + command + '\nquit\n')

This worked in bluetootctl for bluez 5.46 but no longer works in 5.48.

TEST CASE:

echo 'list^Mshow' | bluetoothctl

where ^M can be typed as either carriage return (Ctrl-V Ctrl-M) or line feed (Ctrl-V Ctrl-J).

Expected: Both commands 'list' and 'show' are executed.
Observed: Only 'list' is executed in bluez 5.48.
Comment 1 Luiz Von Dentz 2018-02-19 11:01:49 UTC
This might be cause by the use of wordexp:

'The string argument
       Since the expansion is the same as the expansion by the shell (see sh(1)) of the parameters to a command, the string  s  must
       not  contain  characters  that  would be illegal in shell command parameters.  In particular, there must not be any unescaped
       newline or |, &, ;, <, >, (, ), {, } characters outside a command substitution or parameter substitution context.'

Though that should have caused the whole input to be invalid so I wonder if just the string split not working with ^M for some reason, either way in the long term we are planning to have proper support for non-interactive mode to make it easier to interface bluetoothctl with other tools.
Comment 2 Daniel van Vugt 2018-02-20 03:14:49 UTC
This probably doesn't help much, but I've now bisected the regression for completeness:

70b8b754f8e6f9abe9211c686b279dbef16bf666 is the first bad commit
commit 70b8b754f8e6f9abe9211c686b279dbef16bf666
Author: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date:   Thu Nov 9 16:29:39 2017 +0200

    client: Make use of bt_shell
    
    Use bt_shell instead of readline directly.

:040000 040000 1cf4f1d590b6cf0ad0736d81f1c94a8c21e49c42 787d8c928eb0a06971293deb48195531fa300ccc M	client
Comment 3 Daniel van Vugt 2018-02-20 05:09:43 UTC
Created attachment 274259 [details]
fix-198819-v1.patch

Here's a fix.
Comment 4 Luiz Von Dentz 2018-02-20 09:18:27 UTC
Created attachment 274277 [details]
Don't process HUP cond before others

Let me know if this works as well.
Comment 5 Daniel van Vugt 2018-02-20 09:33:22 UTC
Yes that seems to work too.
Comment 6 Daniel van Vugt 2018-02-21 06:59:40 UTC
Fixed in commit 1bf0336942fd093a0f8fa890eb026e1dc379f35f on master.

Note You need to log in before you can comment on or make changes to this bug.