[vlc-devel] [PATCH] libaccess_rar_plugin: archive compatibility fixes

[Laurent Aimar]

On Sun, Apr 22, 2012 at 07:46:39PM +0200, John Peterson wrote:
> >From cea7c1763d85cdb44927e12c840d90936657fd98 Mon Sep 17 00:00:00 2001
> From: John Peterson <john.peterson3 at hotmail.com>
> Date: Sun, 22 Apr 2012 06:35:02 +0200
> Subject: [PATCH] libaccess_rar_plugin: ! Read the uncompressed size high word instead of the compressed size high word (some uncompressed archives have the compressed size high word as 0).
> ---
>  modules/access/rar/rar.c |   16 +++++++++-------
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/modules/access/rar/rar.c b/modules/access/rar/rar.c
> index d833117..1c858cc 100644
> --- a/modules/access/rar/rar.c
> +++ b/modules/access/rar/rar.c
> @@ -175,7 +176,7 @@ static int SkipFile(stream_t *s, int *count, rar_file_t ***file,
>      uint16_t name_size = GetWLE(&peek[7+19]);
>      uint32_t file_size_high = 0;
>      if (hdr->flags & RAR_BLOCK_FILE_HAS_HIGH)
> -        file_size_high = GetDWLE(&peek[7+25]);
> +        file_size_high = GetDWLE(&peek[7+29]);
>      const uint64_t file_size = ((uint64_t)file_size_high << 32) | file_size_low;
 I think it would be better to also use the uncompressed low word.

> >From cea7c1763d85cdb44927e12c840d90936657fd98 Mon Sep 17 00:00:00 2001
> From: John Peterson <john.peterson3 at hotmail.com>
> Date: Sun, 22 Apr 2012 06:35:02 +0200
> Subject: [PATCH] libaccess_rar_plugin: + Support for up to 1001 volume old naming style archive.
> ---
>  modules/access/rar/rar.c |   16 +++++++++-------
>  1 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/modules/access/rar/rar.c b/modules/access/rar/rar.c
> index d833117..1c858cc 100644
> --- a/modules/access/rar/rar.c
> +++ b/modules/access/rar/rar.c
> @@ -283,10 +284,10 @@ typedef struct {
>  static const rar_pattern_t *FindVolumePattern(const char *location)
>  {
>      static const rar_pattern_t patterns[] = {
> -        { ".part1.rar",   "%s.part%.1d.rar", 2,   9 },
> -        { ".part01.rar",  "%s.part%.2d.rar", 2,  99, },
> -        { ".part001.rar", "%s.part%.3d.rar", 2, 999 },
> -        { ".rar",         "%s.r%.2d",        0,  99 },
> +        { ".part1.rar",   "%s.%cart%.1d.rar", 2,   9 },
> +        { ".part01.rar",  "%s.%cart%.2d.rar", 2,  99 },
> +        { ".part001.rar", "%s.%cart%.3d.rar", 2, 999 },
> +        { ".rar",         "%s.%c%.2d",        0, 999 },
>          { NULL, NULL, 0, 0 },
>      };
> 
> @@ -376,7 +378,9 @@ int RarParse(stream_t *s, int *count, rar_file_t ***file)
>          }
>  
>          free(volume_mrl);
> -        if (asprintf(&volume_mrl, pattern->format, volume_base, volume_index) < 0)
> +        if (asprintf(&volume_mrl, pattern->format, volume_base,
> +            pattern->start ? 'p' : ('r' + (volume_index) / 100),
> +            pattern->start ? volume_index : (volume_index) % 100) < 0)
>                  volume_mrl = NULL;
>          free(volume_base);
 Are you sure this old naming scheme apply to the .part*.rar ?

> >From cea7c1763d85cdb44927e12c840d90936657fd98 Mon Sep 17 00:00:00 2001
> From: John Peterson <john.peterson3 at hotmail.com>
> Date: Sun, 22 Apr 2012 06:35:02 +0200
> Subject: [PATCH] libaccess_rar_plugin: + Robust to non-specification header: No end of archive block despite multi-volume archive.
> ---
>  modules/access/rar/rar.c |   16 +++++++++-------
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/modules/access/rar/rar.c b/modules/access/rar/rar.c
> index d833117..1c858cc 100644
> --- a/modules/access/rar/rar.c
> +++ b/modules/access/rar/rar.c
> @@ -342,6 +343,7 @@ int RarParse(stream_t *s, int *count, rar_file_t ***file)
>                  break;
>              case RAR_BLOCK_FILE:
>                  ret = SkipFile(vol, count, file, &bk, volume_mrl);
> +                has_next = bk.flags & RAR_BLOCK_FILE_HAS_NEXT;
 Sadly, the RAR_BLOCK_FILE_HAS_NEXT flag is not reliable and should
not be checked.
 The current way has_next is computed should be enough to avoid errors
(we check weither or not last file is complete I think).

> >From cea7c1763d85cdb44927e12c840d90936657fd98 Mon Sep 17 00:00:00 2001
> From: John Peterson <john.peterson3 at hotmail.com>
> Date: Sun, 22 Apr 2012 06:35:02 +0200
> Subject: [PATCH] libaccess_rar_plugin: + Robust to non-specification header: Missing added block size flag.
> ---
>  modules/access/rar/rar.c |   16 +++++++++-------
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/modules/access/rar/rar.c b/modules/access/rar/rar.c
> index d833117..1c858cc 100644
> --- a/modules/access/rar/rar.c
> +++ b/modules/access/rar/rar.c
> @@ -66,6 +66,7 @@ enum {
>      RAR_BLOCK_MARKER = 0x72,
>      RAR_BLOCK_ARCHIVE = 0x73,
>      RAR_BLOCK_FILE = 0x74,
> +    RAR_BLOCK_SUBBLOCK = 0x7a,
>      RAR_BLOCK_END = 0x7b,
>  };
>  enum {
> @@ -90,7 +91,7 @@ static int PeekBlock(stream_t *s, rar_block_t *hdr)
>      hdr->flags = GetWLE(&peek[3]);
>      hdr->size  = GetWLE(&peek[5]);
>      hdr->add_size = 0;
> -    if (hdr->flags & 0x8000) {
> +    if (hdr->type == RAR_BLOCK_FILE || hdr->type == RAR_BLOCK_SUBBLOCK) {
>          if (peek_size < 11)
>              return VLC_EGENERIC;
>          hdr->add_size = GetDWLE(&peek[7]);
 I don't see how that could work: the add_size may not always be there weither
or not it's a file or subblock block, no?

Regards,

-- 
fenrir