obexd: Fix can't receive small files sent by windows
PATCH v4 1/1 obexd: Fix can't receive small files sent by windows
When obexd receives a file, it parses the filename and other parameters when processing the packet for the first time, store filename in obex_session and emit the dbus signal, The signal will be pending first. then when this file is received, transfer_complete reset obex_session is called.
When using a computer with Windows 10 installed to send a file to bluez, obexd will read the data through read_stream; if it is a small file, the data processed for the first time is marked as final, and transfer_complete reset obex_session will be called when the data is processed for the first time. At this point, the dbus signal is still pending, and the dbus method that requests the file path has not been processed. This will cause the upper application to not be able to transfer files from the cache directory to the directory specified by the user.
To solve this problem, emit Filename's dbus signal and force it when status=active.
Ways to reproduce the problem:
1. Use the computer with windows 10 installed to send a small file to the computer with ubuntu installed;
2. file size < 10k;
3. After sending, in most cases, the file is located in the ~/.cache/obexd/ directory. Normally, the file should be located in the ~/Download directory.
To fix this, after applying this commit, it also needs to be modified by the upper-level application. Modified to read Filename from dbus signal if there is Filename in dbus signal. Otherwise, use the dbus method to request Filename.
---
v4: fix COMMIT_LOG_LONG_LINE error of checkpatch
obexd/src/manager.c | 6 +++++- obexd/src/obex.c | 1 + 2 files changed, 6 insertions(+), 1 deletion(-)
obexd/src/manager.c +5 -1
@@ -533,8 +533,12 @@ void manager_emit_transfer_property(struct obex_transfer *transfer,void manager_emit_transfer_started (struct obex_transfer *transfer) { transfer->status =TRANSFER_STATUS_ACTIVE ; +if (!transfer->path) +return ; -manager_emit_transfer_property (transfer,"Status" ); +g_dbus_emit_property_changed_full (connection, transfer->path, +TRANSFER_INTERFACE ,"Status" , +G_DBUS_PROPERTY_CHANGED_FLAG_FLUSH ); } static void emit_transfer_completed(struct obex_transfer *transfer,
obexd/src/obex.c +1 -0
@@ -720,6 +720,7 @@ int obex_put_stream_start(struct obex_session *os, const char *filename)manager_emit_transfer_property (os->service_data,"Size" ); os->path =g_strdup (filename); +manager_emit_transfer_property (os->service_data,"Filename" );return 0 ; }
Signature · 1 line
2.20.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=627081
---Test result---
Test Summary:
CheckPatch PASS 1.55 seconds GitLint PASS 1.04 seconds Prep - Setup ELL PASS 55.08 seconds Build - Prep PASS 0.92 seconds Build - Configure PASS 11.05 seconds Build - Make PASS 1990.73 seconds Make Check PASS 13.51 seconds Make Check w/Valgrind PASS 575.57 seconds Make Distcheck PASS 309.50 seconds Build w/ext ELL - Configure PASS 11.60 seconds Build w/ext ELL - Make PASS 1949.50 seconds Incremental Build with patchesPASS 0.00 seconds
---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz
maintainerHi,
Quoted 6 lines · xinpeng wang
When obexd receives a file, it parses the filename and other parameters when processing the packet for the first time, store filename in
obex_sessionand emit the dbus signal, The signal will be pending first. then when this file is received,transfer_completeresetobex_sessionis called.
So if I understand you correctly the problem is that Status does transit directly to complete and end up swallowing the active state? That is not necessarily a bug since the file would still be saved.
Quoted 9 lines
When using a computer with Windows 10 installed to send a file to bluez, obexd will read the data through
read_stream; if it is a small file, the data processed for the first time is marked as final, andtransfer_completeresetobex_sessionwill be called when the data is processed for the first time. At this point, the dbus signal is still pending, and the dbus method that requests the file path has not been processed. This will cause the upper application to not be able to transfer files from the cache directory to the directory specified by the user.
This doesn't sound right, we have chk_put callback that calls into manager_request_authorization and that blocks untils the agent->auth_pending responds, so it sounds like the agent is not responding in time or windows is using ftp instead of opp? Well ftp doesn't even use AuthorizePush, I'm also confused about the mention of a cache directory and user specified directory, there are no such things there only the root directory option which is used as default location and AuthorizePush can opt to store in a different location if necessary, but the transfer doesn't start until the agent reply AuthorizePush or at least that what the code assumes with:
/* Workaround: process events while agent doesn't reply */ while (agent && agent->auth_pending) g_main_context_iteration(NULL, TRUE);
Quoted 10 lines
To solve this problem, emit Filename's dbus signal and force it when status=active.
Ways to reproduce the problem:
1. Use the computer with windows 10 installed to send a small file to the computer with ubuntu installed;
2. file size < 10k;
3. After sending, in most cases, the file is located in the ~/.cache/obexd/ directory. Normally, the file should be located in the ~/Download directory.
Is ~/.cache/obexd its passed as -r/--root to obexd? Could it be that the agent doesn't respond for some reason? e.g. the transfer status is not active?
Quoted 4 lines
To fix this, after applying this commit, it also needs to be modified by the upper-level application. Modified to read Filename from dbus signal if there is Filename in dbus signal. Otherwise, use the dbus method to request Filename.
The method is used to accept the request, so something else is broken if the file ends up being accepted before the user is able to accept it, also it is probably a security breach if small files can bypass the user authorization.
Quoted 42 lines
---
v4: fixCOMMIT_LOG_LONG_LINEerror of checkpatchobexd/src/manager.c | 6 +++++- obexd/src/obex.c | 1 + 2 files changed, 6 insertions(+), 1 deletion(-)obexd/src/manager.c +5
-1@@ -533,8 +533,12 @@ void manager_emit_transfer_property(struct obex_transfer *transfer,void manager_emit_transfer_started (struct obex_transfer *transfer) { transfer->status =TRANSFER_STATUS_ACTIVE ; +if (!transfer->path) +return ; -manager_emit_transfer_property (transfer,"Status" ); +g_dbus_emit_property_changed_full (connection, transfer->path, +TRANSFER_INTERFACE ,"Status" , +G_DBUS_PROPERTY_CHANGED_FLAG_FLUSH ); } static void emit_transfer_completed(struct obex_transfer *transfer,obexd/src/obex.c +1
-1@@ -720,6 +720,7 @@ int obex_put_stream_start(struct obex_session *os, const char *filename)manager_emit_transfer_property (os->service_data,"Size" ); os->path =g_strdup (filename); +manager_emit_transfer_property (os->service_data,"Filename" );return 0 ; } --2.20.1
Signature · 1 line
Luiz Augusto von Dentz
From: Xinpeng Wang <[email protected]>
Hi, Luiz,
Thank you for your reply. I just started to study BlueZ. After receiving
your reply, I reread the code and tested it again with Ubuntu.
Quoted 10 lines · Luiz Augusto von Dentz
Hi,
Quoted 6 lines · xinpeng wang
When obexd receives a file, it parses the filename and other parameters when processing the packet for the first time, store filename in
obex_sessionand emit the dbus signal, The signal will be pending first. then when this file is received,transfer_completeresetobex_sessionis called.
Quoted 3 lines
So if I understand you correctly the problem is that Status does transit directly to complete and end up swallowing the active state? That is not necessarily a bug since the file would still be saved.
First of all, I think this is really a bug. Transfer small file from a Windows 10 computer to a Linux computer with a graphical interface via Bluetooth. After the transfer, the file cannot be found in the ~/Downloads directory of the Linux computer, but the Linux computer has a prompt that the file is in the ~/Downloads directory.
Quoted 11 lines
Quoted 10 lines
When using a computer with Windows 10 installed to send a file to bluez, obexd will read the data through
read_stream; if it is a small file, the data processed for the first time is marked as final, andtransfer_completeresetobex_sessionwill be called when the data is processed for the first time. At this point, the dbus signal is still pending, and the dbus method that requests the file path has not been processed. This will cause the upper application to not be able to transfer files from the cache directory to the directory specified by the user.
Quoted 4 lines
This doesn't sound right, we have
chk_putcallback that calls intomanager_request_authorizationand that blocks untils theagent->auth_pendingresponds, so it sounds like the agent is not responding in time or windows is using ftp instead of opp? Well ftp
Windows uses opp, add use AuthorizePush.
Quoted 4 lines
doesn't even use AuthorizePush, I'm also confused about the mention of a cache directory and user specified directory, there are no such things there only the root directory option which is used as default location and AuthorizePush can opt to store in a different location if
The cache directory is "~/Cache/obexd". In my test, Authorizepush always returns this directory; The user specified directory refers to the directory specified by the user on the graphical interface, for example, in ubuntu, Setting - Bluetooth, there will be a prompt "transferred files are placed in the XXXX folder", default is "~/Downloads", You can change it to another directory here. obexd stores the received files in the cached directory, and after transfer completed, the upper-level program mv files to user specified directory.
Quoted 7 lines
necessary, but the transfer doesn't start until the agent reply AuthorizePush or at least that what the code assumes with:
/* Workaround: process events while agent doesn't reply */ while (agent &&
agent->auth_pending)g_main_context_iteration(NULL, TRUE);
I first debug this problem on UOS(a linux distribution), the upper-level application starts to request the filename after receiving the active signal, so there must be this problem.
In ubuntu, first of all i found that it is very difficult to transfer files from windows to ubuntu, because AuthorizePush always returns forbidden directly. After installed blueman, it is possible to transfer files.
I found that ubuntu calls the dbus method to request the name after cmd_connect, so that the session->name, that is, the filename, is obtained in the place you mentioned above, and there is no path. But in my test, ubuntu does not mv the small file to the ~/Downloads directory in most cases, and it will be fine after applying my patch. I don't understand the upper-level program of ubuntu. I suspect it is related to the active signal?
Quoted 11 lines
Quoted 10 lines
To solve this problem, emit Filename's dbus signal and force it when status=active.
Ways to reproduce the problem:
1. Use the computer with windows 10 installed to send a small file to the computer with ubuntu installed;
2. file size < 10k;
3. After sending, in most cases, the file is located in the ~/.cache/obexd/ directory. Normally, the file should be located in the ~/Download directory.
Quoted 4 lines
Is ~/.cache/obexd its passed as -r/--root to obexd? Could it be that the agent doesn't respond for some reason? e.g. the transfer status is not active?
"~/.cache/obexd" and agent are normal.
Quoted 10 lines
Quoted 4 lines
To fix this, after applying this commit, it also needs to be modified by the upper-level application. Modified to read Filename from dbus signal if there is Filename in dbus signal. Otherwise, use the dbus method to request Filename.
The method is used to accept the request, so something else is broken if the file ends up being accepted before the user is able to accept it, also it is probably a security breach if small files can bypass the user authorization.
This patch won't have the security issues you mentioned, because the added signals are all after AuthorizePush returns successfully. My test is that before AuthorizePush returns or returns fail, no signal will be emitted and file transfer will not start.
The method is used to request the file name and path. What I mean here is that for the upper-level program, if there is a Filename in the dbus signal, use this value, otherwise, call the dbus method to get it as before. In ubuntu (without this patch), the file name is obtained before AuthorizePush returns.
Quoted 47 lines
Quoted 42 lines
---
v4: fixCOMMIT_LOG_LONG_LINEerror of checkpatchobexd/src/manager.c | 6 +++++- obexd/src/obex.c | 1 + 2 files changed, 6 insertions(+), 1 deletion(-)obexd/src/manager.c +5
-1@@ -533,8 +533,12 @@ void manager_emit_transfer_property(struct obex_transfer *transfer,void manager_emit_transfer_started (struct obex_transfer *transfer) { transfer->status =TRANSFER_STATUS_ACTIVE ; +if (!transfer->path) +return ; -manager_emit_transfer_property (transfer,"Status" ); +g_dbus_emit_property_changed_full (connection, transfer->path, +TRANSFER_INTERFACE ,"Status" , +G_DBUS_PROPERTY_CHANGED_FLAG_FLUSH ); } static void emit_transfer_completed(struct obex_transfer *transfer,obexd/src/obex.c +1
-1@@ -720,6 +720,7 @@ int obex_put_stream_start(struct obex_session *os, const char *filename)manager_emit_transfer_property (os->service_data,"Size" ); os->path =g_strdup (filename); +manager_emit_transfer_property (os->service_data,"Filename" );return 0 ; } --2.20.1
Signature · 1 line
Luiz Augusto von Dentz