2022-03-25 17:28:54

by Paul Menzel

[permalink] [raw]
Subject: Re: [PATCH v2] obexd: Fix can't receive small files sent by windows

Dear Xinpeng,


Am 25.03.22 um 08:13 schrieb 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_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.
>
> Signed-off-by: xinpeng wang <[email protected]>
> ---
> v2: Add the steps to reproduce the bug in the submission information.
>
> obexd/src/manager.c | 6 +++++-
> obexd/src/obex.c | 1 +
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/obexd/src/manager.c b/obexd/src/manager.c
> index 01741fe62..2c180dc44 100644
> --- a/obexd/src/manager.c
> +++ b/obexd/src/manager.c
> @@ -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,
> diff --git a/obexd/src/obex.c b/obexd/src/obex.c
> index 3a68fd66c..c0d9e160a 100644
> --- a/obexd/src/obex.c
> +++ b/obexd/src/obex.c
> @@ -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;
> }

Awesome. Thank you.

Acked-by: Paul Menzel <[email protected]>


Kind regards,

Paul


2022-03-25 20:27:36

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2] obexd: Fix can't receive small files sent by windows

Hi Paul, Xinpeng,

On Fri, Mar 25, 2022 at 10:28 AM Paul Menzel <[email protected]> wrote:
>
> Dear Xinpeng,
>
>
> Am 25.03.22 um 08:13 schrieb 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_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.

It would be great to have the obexctl output with and without the
change, also does it change anything with respect of bigger files that
are not completed with a single packet?

> > 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.
> >
> > Signed-off-by: xinpeng wang <[email protected]>

We don't use Signed-off-by for userspace.

> > ---
> > v2: Add the steps to reproduce the bug in the submission information.
> >
> > obexd/src/manager.c | 6 +++++-
> > obexd/src/obex.c | 1 +
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/obexd/src/manager.c b/obexd/src/manager.c
> > index 01741fe62..2c180dc44 100644
> > --- a/obexd/src/manager.c
> > +++ b/obexd/src/manager.c
> > @@ -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,
> > diff --git a/obexd/src/obex.c b/obexd/src/obex.c
> > index 3a68fd66c..c0d9e160a 100644
> > --- a/obexd/src/obex.c
> > +++ b/obexd/src/obex.c
> > @@ -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;
> > }
>
> Awesome. Thank you.
>
> Acked-by: Paul Menzel <[email protected]>
>
>
> Kind regards,
>
> Paul



--
Luiz Augusto von Dentz