Minor fixes and X-Archive-Charset + Unicode Path patch

[tony2001]

Hello Evan.

I've added a crude implementation of Unicode Path extra field along with ability to control the charset of original filenames.
As far as I understand, that exactly what the person here: https://github.com/evanmiller/mod_zip/issues#issue/3 is asking for.
The only problem now is that the Unicode Path thing IS added and the archive is correctly opened by unzip and arK, BUT I still can't see Unicode filename, so it either doesn't really work, or unzip/arK ignore it for some reason.
I would appreciate if you look at the patches and (hopefully) pull them if they look good for you. If they don't - any comment would be still appreciated.

Thanks in advance.

[evanmiller]

Hi Tony,

Very cool! This looks really good. A couple comments:

1. Can you add a couple tests to the test suite found in t/ so that this code doesn't break in the future?
2. I am hesitant to add a library dependency (iconv). Do you know whether iconv standard on all systems that Nginx compiles on? If not, can we #ifdef around it?

[tony2001]

Ok, I'll see if I can add the tests today.
The iconv is actually is a part of glibc on Linux, so it's a library dependency only on FreeBSD and the like.

[evanmiller]

Thanks.

On FreeBSD iconv depends (in turn) on Perl. I would prefer that mod_zip not depend on having a Perl installation.

I know Nginx has its own charset conversion facilities (used in the Charset module). Although they are primitive, perhaps they could be used as a fallback when iconv is not present.

[tony2001]

On FreeBSD iconv depends (in turn) on Perl.
I don't use FreeBSD myself, but this sounds wrong:
http://www.freebsd.org/ports/converters.html#libiconv-1.13.1_1
Requires: libtool-2.2.10 - that's the only dependency I can see.

[tony2001]

Well, it seems that to test this thing I need to be able to extract files with names in different encodings. It does work if I use UTF8, but that's kind of expected, isn't it?

But if I use, say CP866 (which is used by default in russian edition of MS Windows, which in turn is the reason of all this mess), then Perl::Zip is unable to read the filenames in the archive correctly.

To be honest, even unzip-6.0 shows lots of "?????" instead of cyrillic letters.
The only app that seems to support Unicode Path correctly is unzip 6.10a, which does extract all the files correctly.

[evanmiller]

Ah, I was looking at the "iconv" utilities package -- my bad.

So then depending on libiconv is OK with me, as long as you update the "config" file to test for it during configure. See here for an example that tests for libMagickWand:

https://github.com/evanmiller/nginx_circle_gif/blob/master/config

I don't mind requiring unzip 6.10a for the test suite, as long as your print a warning if it's not present.

[tony2001]

The point is that on Linux libiconv is not needed at all.
Is there a way to make a "conditional" dependency in the config?

[evanmiller]

Hi Tony,

OK, I think I have a start. If you add this to config:

ngx_feature="iconv_open()"
ngx_feature_name="NGX_ZIP_HAVE_ICONV"
ngx_feature_run=no
ngx_feature_incs="#include <iconv.h>"
ngx_feature_path=
ngx_feature_libs=
ngx_feature_test="iconv_open("IBM-850", "ISO8859-1");"
. auto/feature

...then NGX_ZIP_HAVE_ICONV will be defined if iconv is present (on Linux... but not in library form). If we then #ifdef NGX_ZIP_HAVE_ICONV around the iconv stuff, then stuff should "just work" out of the box for everyone. Still need to figure out how to test the platform and then add:

ngx_feature_libs=-liconv

...if we're not running on Linux.

[evanmiller]

I think this config file will do the trick:

ngx_addon_name=ngx_http_zip_module
HTTP_AUX_FILTER_MODULES="$HTTP_AUX_FILTER_MODULES ngx_http_zip_module"
NGX_ADDON_SRCS="$NGX_ADDON_SRCS $ngx_addon_dir/ngx_http_zip_module.c"
NGX_ADDON_SRCS="$NGX_ADDON_SRCS $ngx_addon_dir/ngx_http_zip_parsers.c"
NGX_ADDON_SRCS="$NGX_ADDON_SRCS $ngx_addon_dir/ngx_http_zip_file.c"
NGX_ADDON_SRCS="$NGX_ADDON_SRCS $ngx_addon_dir/ngx_http_zip_headers.c"

ngx_feature="iconv_open()"
ngx_feature_name="NGX_ZIP_HAVE_ICONV"
ngx_feature_run=no
ngx_feature_incs="#include <iconv.h>"
ngx_feature_path=
case "$NGX_PLATFORM" in
    Linux:*)
        ngx_feature_libs=
    ;;

    *)
        ngx_feature_libs="-liconv"
    ;;
esac
ngx_feature_test="iconv_open(\"IBM-850\", \"ISO8859-1\");"
. auto/feature

[evanmiller]

I have pulled in your changes and #ifdef'd around the iconv stuff so mod_zip always compiles. You should test out what I did. If everything looks OK, this will ship in the next version.

[tony2001]

Thanks a lot, I'll take a look.