On Sun, Feb 20, 2011 at 09:24:48PM +0300, Maxim Dounin wrote:
> [...]
>
> 1. You shouldn't use ngx_alloc() for request-related data. Using
> ngx_palloc() is much easier and don't require free() (which is
> missed in your patch, btw).
>
Oups yeah sorry for the free, I should have spotted that. Ok for
ngx_palloc.
> 2. There is no need to do any separate allocs in this case,
> changing main one (and relevant output code) in
> ngx_http_autoindex_handler() is enough.
>
Ok, patch updated to behave like this.
> Additionally, I personally believe that using css without making
> sure page will be rendered in standards mode is bad idea.
>
I added a patch to make it HTML compliant (with a few warnings, but it's
valid). I don't if it is really what you wanted though.
> > Also, I didn't add the prototype declaration since it's a static function;
> > should I?
>
> Generally style suggest that handler function should be kept
> first, and static functions are declared. Following any of these
> rules will require you to add declaration.
>
Ok.
> Though in this particular case there is no need to add
> function, see above.
>
Function removed.
> [...]
Thank for your review. Patches re-attached. I don't know the commit
politic here about mixing reindent so I made a different one for
re-alignement. If I have to squash them I'll resend the patches.
Regards,
--
Clément B.
From 5798a8c4c67f429229a414856d6507470cad8b70 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux@gmail.com>
Date: Sun, 20 Feb 2011 17:57:03 +0100
Subject: [PATCH 1/3] Add autoindex_css_file option.
---
src/http/modules/ngx_http_autoindex_module.c | 39 ++++++++++++++++++++++++--
1 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/src/http/modules/ngx_http_autoindex_module.c b/src/http/modules/ngx_http_autoindex_module.c
index b679318..76788f4 100644
--- a/src/http/modules/ngx_http_autoindex_module.c
+++ b/src/http/modules/ngx_http_autoindex_module.c
@@ -39,6 +39,7 @@ typedef struct {
ngx_flag_t enable;
ngx_flag_t localtime;
ngx_flag_t exact_size;
+ ngx_str_t css_file;
} ngx_http_autoindex_loc_conf_t;
@@ -80,6 +81,13 @@ static ngx_command_t ngx_http_autoindex_commands[] = {
offsetof(ngx_http_autoindex_loc_conf_t, exact_size),
NULL },
+ { ngx_string("autoindex_css_file"),
+ NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
+ ngx_conf_set_str_slot,
+ NGX_HTTP_LOC_CONF_OFFSET,
+ offsetof(ngx_http_autoindex_loc_conf_t, css_file),
+ NULL },
+
ngx_null_command
};
@@ -120,6 +128,12 @@ static u_char title[] =
"<head><title>Index of "
;
+static u_char title_pre_css[] =
+"<html>" CRLF
+"<head><link rel=\"stylesheet\" type=\"text/css\" href=\""
+;
+
+static u_char title_post_css[] = "\" /><title>Index of ";
static u_char header[] =
"</title></head>" CRLF
@@ -358,8 +372,15 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
ngx_close_dir_n " \"%s\" failed", &path);
}
- len = sizeof(title) - 1
- + r->uri.len
+ if (!alcf->css_file.len) {
+ len = sizeof(title) - 1;
+ } else {
+ len = sizeof(title_pre_css) - 1
+ + alcf->css_file.len
+ + sizeof(title_post_css) - 1;
+ }
+
+ len += r->uri.len
+ sizeof(header) - 1
+ r->uri.len
+ sizeof("</h1>") - 1
@@ -392,7 +413,16 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
ngx_http_autoindex_cmp_entries);
}
- b->last = ngx_cpymem(b->last, title, sizeof(title) - 1);
+ if (!alcf->css_file.len) {
+ b->last = ngx_cpymem(b->last, title, sizeof(title) - 1);
+ } else {
+ b->last = ngx_cpymem(b->last, title_pre_css,
+ sizeof(title_pre_css) - 1);
+ b->last = ngx_cpymem(b->last, alcf->css_file.data, alcf->css_file.len);
+ b->last = ngx_cpymem(b->last, title_post_css,
+ sizeof(title_post_css) - 1);
+ }
+
b->last = ngx_cpymem(b->last, r->uri.data, r->uri.len);
b->last = ngx_cpymem(b->last, header, sizeof(header) - 1);
b->last = ngx_cpymem(b->last, r->uri.data, r->uri.len);
@@ -633,6 +663,8 @@ ngx_http_autoindex_create_loc_conf(ngx_conf_t *cf)
conf->enable = NGX_CONF_UNSET;
conf->localtime = NGX_CONF_UNSET;
conf->exact_size = NGX_CONF_UNSET;
+ conf->css_file.len = 0;
+ conf->css_file.data = NULL;
return conf;
}
@@ -647,6 +679,7 @@ ngx_http_autoindex_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
ngx_conf_merge_value(conf->enable, prev->enable, 0);
ngx_conf_merge_value(conf->localtime, prev->localtime, 0);
ngx_conf_merge_value(conf->exact_size, prev->exact_size, 1);
+ ngx_conf_merge_str_value(conf->css_file, prev->css_file, "");
return NGX_CONF_OK;
}
--
1.7.4.1
From c49151eb3d3772b9ce564dffbd40e94557426c6d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux@gmail.com>
Date: Sun, 20 Feb 2011 21:24:16 +0100
Subject: [PATCH 2/3] Reindent after previous commit.
---
src/http/modules/ngx_http_autoindex_module.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/http/modules/ngx_http_autoindex_module.c b/src/http/modules/ngx_http_autoindex_module.c
index 76788f4..8d43028 100644
--- a/src/http/modules/ngx_http_autoindex_module.c
+++ b/src/http/modules/ngx_http_autoindex_module.c
@@ -381,12 +381,12 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
}
len += r->uri.len
- + sizeof(header) - 1
- + r->uri.len
- + sizeof("</h1>") - 1
- + sizeof("<hr><pre><a href=\"../\">../</a>" CRLF) - 1
- + sizeof("</pre><hr>") - 1
- + sizeof(tail) - 1;
+ + sizeof(header) - 1
+ + r->uri.len
+ + sizeof("</h1>") - 1
+ + sizeof("<hr><pre><a href=\"../\">../</a>" CRLF) - 1
+ + sizeof("</pre><hr>") - 1
+ + sizeof(tail) - 1;
entry = entries.elts;
for (i = 0; i < entries.nelts; i++) {
--
1.7.4.1
From be819305df1dac8c5f6499e0016bb370f3059247 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux@gmail.com>
Date: Sun, 20 Feb 2011 21:27:44 +0100
Subject: [PATCH 3/3] Use valid HTML.
---
src/http/modules/ngx_http_autoindex_module.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/http/modules/ngx_http_autoindex_module.c b/src/http/modules/ngx_http_autoindex_module.c
index 8d43028..1cb136b 100644
--- a/src/http/modules/ngx_http_autoindex_module.c
+++ b/src/http/modules/ngx_http_autoindex_module.c
@@ -124,12 +124,12 @@ ngx_module_t ngx_http_autoindex_module = {
static u_char title[] =
-"<html>" CRLF
+"<!DOCTYPE html>" CRLF
"<head><title>Index of "
;
static u_char title_pre_css[] =
-"<html>" CRLF
+"<!DOCTYPE html>" CRLF
"<head><link rel=\"stylesheet\" type=\"text/css\" href=\""
;
@@ -137,7 +137,7 @@ static u_char title_post_css[] = "\" /><title>Index of ";
static u_char header[] =
"</title></head>" CRLF
-"<body bgcolor=\"white\">" CRLF
+"<body>" CRLF
"<h1>Index of "
;
@@ -384,8 +384,8 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
+ sizeof(header) - 1
+ r->uri.len
+ sizeof("</h1>") - 1
- + sizeof("<hr><pre><a href=\"../\">../</a>" CRLF) - 1
- + sizeof("</pre><hr>") - 1
+ + sizeof("<hr/><pre><a href=\"../\">../</a>" CRLF) - 1
+ + sizeof("</pre><hr/>") - 1
+ sizeof(tail) - 1;
entry = entries.elts;
@@ -428,8 +428,8 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
b->last = ngx_cpymem(b->last, r->uri.data, r->uri.len);
b->last = ngx_cpymem(b->last, "</h1>", sizeof("</h1>") - 1);
- b->last = ngx_cpymem(b->last, "<hr><pre><a href=\"../\">../</a>" CRLF,
- sizeof("<hr><pre><a href=\"../\">../</a>" CRLF) - 1);
+ b->last = ngx_cpymem(b->last, "<hr/><pre><a href=\"../\">../</a>" CRLF,
+ sizeof("<hr/><pre><a href=\"../\">../</a>" CRLF) - 1);
tp = ngx_timeofday();
@@ -563,7 +563,7 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
/* TODO: free temporary pool */
- b->last = ngx_cpymem(b->last, "</pre><hr>", sizeof("</pre><hr>") - 1);
+ b->last = ngx_cpymem(b->last, "</pre><hr/>", sizeof("</pre><hr/>") - 1);
b->last = ngx_cpymem(b->last, tail, sizeof(tail) - 1);
--
1.7.4.1
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://nginx.org/mailman/listinfo/nginx-devel
