Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Fix file URI handling when deleting files.
On 11/29/18 11:49 PM, Andrey Semashev wrote: > On 11/29/18 2:17 PM, Andrey Semashev wrote: >> On 11/29/18 2:15 PM, Andrey Semashev wrote: >>> On 11/28/18 7:47 PM, Jeyapal, Karthick wrote: On 11/28/18 4:46 PM, Andrey Semashev wrote: > The URI used to open the output streams may be an actual URI with "file" > scheme, > according to https://tools.ietf.org/html/rfc8089. This commit makes file > deletion routine recognize file URIs and extract the actual filesystem > path > from it. There is already some code in ffmpeg to handle this. It is present in file_delete() function in file.c. We will need to avoid code duplication for the same functionality. One option could be to call avpriv_io_delete() function instead of calling unlink, so that file_delete function gets called. Calling avpriv_io_delete will also make the delete functionality easily extendable for other output protocols. >>> >>> That would be fine with me, but I'm using Linux. Looking at file_delete (in >>> libavformat/file.c), it looks like it will only work on POSIX systems but >>> not on Windows, since it doesn't have unistd.h. Am I correct? And if so, is >>> avpriv_io_delete still the preferred approach? >> >> Also, that code doesn't seem to support the URI with an authority field and >> doesn't check the special "localhost" case. > > I've sent a new set of patches that updates both file.c and dashenc.c. Thanks for your understanding. Looks like that will be the clean approach for fixing this problem. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Fix file URI handling when deleting files.
On 11/29/18 2:17 PM, Andrey Semashev wrote: On 11/29/18 2:15 PM, Andrey Semashev wrote: On 11/28/18 7:47 PM, Jeyapal, Karthick wrote: On 11/28/18 4:46 PM, Andrey Semashev wrote: The URI used to open the output streams may be an actual URI with "file" scheme, according to https://tools.ietf.org/html/rfc8089. This commit makes file deletion routine recognize file URIs and extract the actual filesystem path from it. There is already some code in ffmpeg to handle this. It is present in file_delete() function in file.c. We will need to avoid code duplication for the same functionality. One option could be to call avpriv_io_delete() function instead of calling unlink, so that file_delete function gets called. Calling avpriv_io_delete will also make the delete functionality easily extendable for other output protocols. That would be fine with me, but I'm using Linux. Looking at file_delete (in libavformat/file.c), it looks like it will only work on POSIX systems but not on Windows, since it doesn't have unistd.h. Am I correct? And if so, is avpriv_io_delete still the preferred approach? Also, that code doesn't seem to support the URI with an authority field and doesn't check the special "localhost" case. I've sent a new set of patches that updates both file.c and dashenc.c. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Fix file URI handling when deleting files.
On 11/29/18 2:15 PM, Andrey Semashev wrote: On 11/28/18 7:47 PM, Jeyapal, Karthick wrote: On 11/28/18 4:46 PM, Andrey Semashev wrote: The URI used to open the output streams may be an actual URI with "file" scheme, according to https://tools.ietf.org/html/rfc8089. This commit makes file deletion routine recognize file URIs and extract the actual filesystem path from it. There is already some code in ffmpeg to handle this. It is present in file_delete() function in file.c. We will need to avoid code duplication for the same functionality. One option could be to call avpriv_io_delete() function instead of calling unlink, so that file_delete function gets called. Calling avpriv_io_delete will also make the delete functionality easily extendable for other output protocols. That would be fine with me, but I'm using Linux. Looking at file_delete (in libavformat/file.c), it looks like it will only work on POSIX systems but not on Windows, since it doesn't have unistd.h. Am I correct? And if so, is avpriv_io_delete still the preferred approach? Also, that code doesn't seem to support the URI with an authority field and doesn't check the special "localhost" case. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Fix file URI handling when deleting files.
On 11/28/18 7:47 PM, Jeyapal, Karthick wrote: On 11/28/18 4:46 PM, Andrey Semashev wrote: The URI used to open the output streams may be an actual URI with "file" scheme, according to https://tools.ietf.org/html/rfc8089. This commit makes file deletion routine recognize file URIs and extract the actual filesystem path from it. There is already some code in ffmpeg to handle this. It is present in file_delete() function in file.c. We will need to avoid code duplication for the same functionality. One option could be to call avpriv_io_delete() function instead of calling unlink, so that file_delete function gets called. Calling avpriv_io_delete will also make the delete functionality easily extendable for other output protocols. That would be fine with me, but I'm using Linux. Looking at file_delete (in libavformat/file.c), it looks like it will only work on POSIX systems but not on Windows, since it doesn't have unistd.h. Am I correct? And if so, is avpriv_io_delete still the preferred approach? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Fix file URI handling when deleting files.
On 11/28/18 4:46 PM, Andrey Semashev wrote:
> The URI used to open the output streams may be an actual URI with "file"
> scheme,
> according to https://tools.ietf.org/html/rfc8089. This commit makes file
> deletion routine recognize file URIs and extract the actual filesystem path
> from it.
There is already some code in ffmpeg to handle this. It is present in
file_delete() function in file.c.
We will need to avoid code duplication for the same functionality. One option
could be to call avpriv_io_delete() function instead of calling unlink, so that
file_delete function gets called.
Calling avpriv_io_delete will also make the delete functionality easily
extendable for other output protocols.
>
> It also fixes strerror use, which may not be thread-safe.
> ---
> libavformat/dashenc.c | 29 +++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> index 6ce70e0076..e59fa0944e 100644
> --- a/libavformat/dashenc.c
> +++ b/libavformat/dashenc.c
> @@ -25,6 +25,7 @@
> #include
> #endif
>
> +#include "libavutil/error.h"
> #include "libavutil/avassert.h"
> #include "libavutil/avutil.h"
> #include "libavutil/avstring.h"
> @@ -1326,8 +1327,32 @@ static void dashenc_delete_file(AVFormatContext *s,
> char *filename) {
>
> av_dict_free(&http_opts);
> ff_format_io_close(s, &out);
> -} else if (unlink(filename) < 0) {
> -av_log(s, AV_LOG_ERROR, "failed to delete %s: %s\n", filename,
> strerror(errno));
> +} else {
> +const char* path = filename;
> +// Check if the filename is a file URI.
> https://tools.ietf.org/html/rfc8089#section-2
> +if (av_strncasecmp(path, "file:", sizeof("file:") - 1) == 0) {
> +path += sizeof("file:") - 1;
> +if (path[0] == '/' && path[1] == '/') {
> +// The URI may have an authority part. Check that the
> authority does not contain
> +// a host name. We cannot access filesystem on a different
> host.
> +path += 2;
> +if (path[0] != '/') {
> +if (strncmp(path, "localhost", sizeof("localhost") - 1)
> == 0) {
> +path += sizeof("localhost") - 1;
> +} else {
> +av_log(s, AV_LOG_ERROR, "cannot delete file on a
> remote host: %s\n", filename);
> +return;
> +}
> +}
> +}
> +}
> +
> +if (unlink(path) < 0) {
> +int err = AVERROR(errno);
> +char errbuf[128];
> +av_strerror(err, errbuf, sizeof(errbuf));
> +av_log(s, (err == AVERROR(ENOENT) ? AV_LOG_WARNING :
> AV_LOG_ERROR), "failed to delete %s: %s\n", path, errbuf);
> +}
> }
> }
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/dashenc: Fix file URI handling when deleting files.
The URI used to open the output streams may be an actual URI with "file" scheme,
according to https://tools.ietf.org/html/rfc8089. This commit makes file
deletion routine recognize file URIs and extract the actual filesystem path
from it.
It also fixes strerror use, which may not be thread-safe.
---
libavformat/dashenc.c | 29 +++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6ce70e0076..e59fa0944e 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -25,6 +25,7 @@
#include
#endif
+#include "libavutil/error.h"
#include "libavutil/avassert.h"
#include "libavutil/avutil.h"
#include "libavutil/avstring.h"
@@ -1326,8 +1327,32 @@ static void dashenc_delete_file(AVFormatContext *s, char
*filename) {
av_dict_free(&http_opts);
ff_format_io_close(s, &out);
-} else if (unlink(filename) < 0) {
-av_log(s, AV_LOG_ERROR, "failed to delete %s: %s\n", filename,
strerror(errno));
+} else {
+const char* path = filename;
+// Check if the filename is a file URI.
https://tools.ietf.org/html/rfc8089#section-2
+if (av_strncasecmp(path, "file:", sizeof("file:") - 1) == 0) {
+path += sizeof("file:") - 1;
+if (path[0] == '/' && path[1] == '/') {
+// The URI may have an authority part. Check that the
authority does not contain
+// a host name. We cannot access filesystem on a different
host.
+path += 2;
+if (path[0] != '/') {
+if (strncmp(path, "localhost", sizeof("localhost") - 1) ==
0) {
+path += sizeof("localhost") - 1;
+} else {
+av_log(s, AV_LOG_ERROR, "cannot delete file on a
remote host: %s\n", filename);
+return;
+}
+}
+}
+}
+
+if (unlink(path) < 0) {
+int err = AVERROR(errno);
+char errbuf[128];
+av_strerror(err, errbuf, sizeof(errbuf));
+av_log(s, (err == AVERROR(ENOENT) ? AV_LOG_WARNING :
AV_LOG_ERROR), "failed to delete %s: %s\n", path, errbuf);
+}
}
}
--
2.19.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
