vdr 1.3.38: SVDRP grab filename

  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):


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.