Hi,
as far as I see, the new SVDRP grab code always prepends the given filename with the grabdirectory (given with -g) even if the filename in the grab command is actualy a full path.
Is this change intended?
I yes, may be this should be documented somewhere, as it's not backward compatible with older vdr versions. But I don't see a reason why it's necessary. The path check later will catch a wrong path anyways.
I'd suggested something like (defaulting to jpeg with no filename extention for enhanced backward compatibility too):
--- svdrp.c.orig 2005-12-30 16:42:29.000000000 +0100 +++ svdrp.c 2006-01-09 19:15:51.000000000 +0100 @@ -682,8 +682,7 @@ else if (strcmp(FileName, "-") == 0) FileName = NULL; else { - Reply(501, "Missing filename extension in "%s"", FileName); - return; + Jpeg = true; } // image quality (and obsolete type): if ((p = strtok_r(NULL, delim, &strtok_next)) != NULL) { @@ -729,10 +728,13 @@ char RealFileName[PATH_MAX]; if (FileName) { if (grabImageDir) { - char *s; - asprintf(&s, "%s/%s", grabImageDir, FileName); - FileName = s; - char *slash = strrchr(FileName, '/'); // there definitely is one + char *s = 0; + char *slash = strrchr(FileName, '/'); + if (!slash) { + asprintf(&s, "%s/%s", grabImageDir, FileName); + FileName = s; + } + slash = strrchr(FileName, '/'); // there definitely is one *slash = 0; char *r = realpath(FileName, RealFileName); *slash = '/';
Regards.
Stefan Huelswitt wrote:
Hi,
as far as I see, the new SVDRP grab code always prepends the given filename with the grabdirectory (given with -g) even if the filename in the grab command is actualy a full path.
Is this change intended?
I yes, may be this should be documented somewhere, as it's not backward compatible with older vdr versions. But I don't see a reason why it's necessary. The path check later will catch a wrong path anyways.
I'd suggested something like (defaulting to jpeg with no filename extention for enhanced backward compatibility too):
--- svdrp.c.orig 2005-12-30 16:42:29.000000000 +0100 +++ svdrp.c 2006-01-09 19:15:51.000000000 +0100 @@ -682,8 +682,7 @@ else if (strcmp(FileName, "-") == 0) FileName = NULL; else {
Reply(501, "Missing filename extension in \"%s\"", FileName);return;
Jpeg = true;
This would allow overwriting files with extensions other than ".jpeg", ".jpg" or ".pnm" - is this really what you want?
} // image quality (and obsolete type): if ((p = strtok_r(NULL, delim, &strtok_next)) != NULL) {@@ -729,10 +728,13 @@ char RealFileName[PATH_MAX]; if (FileName) { if (grabImageDir) {
char *s;asprintf(&s, "%s/%s", grabImageDir, FileName);FileName = s;char *slash = strrchr(FileName, '/'); // there definitely is one
char *s = 0;char *slash = strrchr(FileName, '/');if (!slash) {asprintf(&s, "%s/%s", grabImageDir, FileName);FileName = s;}slash = strrchr(FileName, '/'); // there definitely is one *slash = 0; char *r = realpath(FileName, RealFileName); *slash = '/';
Ok, that sounds reasonable.
Klaus
On 14 Jan 2006 Klaus Schmidinger Klaus.Schmidinger@cadsoft.de wrote:
Stefan Huelswitt wrote:
--- svdrp.c.orig 2005-12-30 16:42:29.000000000 +0100 +++ svdrp.c 2006-01-09 19:15:51.000000000 +0100 @@ -682,8 +682,7 @@ else if (strcmp(FileName, "-") == 0) FileName = NULL; else {
Reply(501, "Missing filename extension in \"%s\"", FileName);return;
Jpeg = true;This would allow overwriting files with extensions other than ".jpeg", ".jpg" or ".pnm" - is this really what you want?
Actualy it allows overwriting files with no extention only.
I find it convinient to be backward compatible. As the whole operation is limited to the grab directory anyways, security considerations are negligible IMHO.
Regards.
Stefan Huelswitt wrote:
On 14 Jan 2006 Klaus Schmidinger Klaus.Schmidinger@cadsoft.de wrote:
Stefan Huelswitt wrote:
--- svdrp.c.orig 2005-12-30 16:42:29.000000000 +0100 +++ svdrp.c 2006-01-09 19:15:51.000000000 +0100 @@ -682,8 +682,7 @@ else if (strcmp(FileName, "-") == 0) FileName = NULL; else {
Reply(501, "Missing filename extension in \"%s\"", FileName);return;
Jpeg = true;This would allow overwriting files with extensions other than ".jpeg", ".jpg" or ".pnm" - is this really what you want?
Actualy it allows overwriting files with no extention only.
I find it convinient to be backward compatible. As the whole operation is limited to the grab directory anyways, security considerations are negligible IMHO.
And what's the point in writing to a file without an extension?
Klaus
On 14 Jan 2006 Klaus Schmidinger Klaus.Schmidinger@cadsoft.de wrote:
Stefan Huelswitt wrote:
Actualy it allows overwriting files with no extention only.
I find it convinient to be backward compatible. As the whole operation is limited to the grab directory anyways, security considerations are negligible IMHO.
And what's the point in writing to a file without an extension?
For me the point was, that vdradmin uses a grab file without extention since ever. vdr 1.3.38 broke that.
I think it's a good idea to behave in a backward compatible way whenever possible.
Jut my 2 cents.
Regards.
Stefan Huelswitt wrote:
On 14 Jan 2006 Klaus Schmidinger Klaus.Schmidinger@cadsoft.de wrote:
Stefan Huelswitt wrote:
Actualy it allows overwriting files with no extention only.
I find it convinient to be backward compatible. As the whole operation is limited to the grab directory anyways, security considerations are negligible IMHO.
And what's the point in writing to a file without an extension?
For me the point was, that vdradmin uses a grab file without extention since ever. vdr 1.3.38 broke that.
I think it's a good idea to behave in a backward compatible way whenever possible.
Jut my 2 cents.
Ok, since there have been no other voices on this, and personally I don't really care, I've removed the entire 'else' clause, since Jpeg is true by default:
--- svdrp.c 2006/01/14 14:55:52 1.92 +++ svdrp.c 2006/01/14 16:08:20 @@ -683,10 +683,6 @@ } else if (strcmp(FileName, "-") == 0) FileName = NULL; - else { - Reply(501, "Missing filename extension in "%s"", FileName); - return; - } // image quality (and obsolete type): if ((p = strtok_r(NULL, delim, &strtok_next)) != NULL) { if (strcasecmp(p, "JPEG") == 0 || strcasecmp(p, "PNM") == 0) {
However, this still doesn't allow names with '.' that don't end in one of the known extensions. But then again I would assume that programs connecting to an SVDRP port will switch to using the base64 encoded data sent over the port.
Klaus
-
Klaus Schmidinger -
s.huelswitt＠gmx.de