r111444 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r111443‎ | r111444 | r111445 >
Date:03:35, 14 February 2012
Author:abemusic
Status:ok (Comments)
Tags:
Comment:
- Adds a "udplog_escaped_content_type" variable for url encoding the content type header
Modified paths:
  • /trunk/debs/nginx/modules/nginx-udplog/ngx_http_udplog_module.c (modified) (history)

Diff [purge]

Index: trunk/debs/nginx/modules/nginx-udplog/ngx_http_udplog_module.c
——@@ -87,6 +87,8 @@
8888 ngx_http_variable_value_t *v, uintptr_t data);
8989 static ngx_int_t ngx_http_udplog_escaped_user_agent_variable(ngx_http_request_t *r,
9090 ngx_http_variable_value_t *v, uintptr_t data);
 91+static ngx_int_t ngx_http_udplog_escaped_content_type_variable(ngx_http_request_t *r,
 92+ ngx_http_variable_value_t *v, uintptr_t data);
9193
9294 static ngx_http_variable_t ngx_http_udplog_variables[] = {
9395 { ngx_string("udplog_time"), NULL, ngx_http_udplog_time_variable, 0,
——@@ -95,6 +97,8 @@
9698 NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_NOHASH, 0 },
9799 { ngx_string("udplog_escaped_user_agent"), NULL, ngx_http_udplog_escaped_user_agent_variable, 0,
98100 NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_NOHASH, 0 },
 101+ { ngx_string("udplog_escaped_content_type"), NULL, ngx_http_udplog_escaped_content_type_variable, 0,
 102+ NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_NOHASH, 0 },
99103
100104 { ngx_null_string, NULL, NULL, 0, 0, 0 }
101105 };
——@@ -242,6 +246,38 @@
243247 }
244248
245249 static ngx_int_t
 250+ngx_http_udplog_escaped_content_type_variable(ngx_http_request_t *r,
 251+ ngx_http_variable_value_t *v, uintptr_t data)
 252+{
 253+ u_char *ct;
 254+ uintptr_t escape;
 255+ size_t l;
 256+
 257+ // Check that the content type string was processed.
 258+ if(r->headers_in.content_type == NULL) {
 259+ return NGX_ERROR;
 260+ }
 261+
 262+ ct = r->headers_in.content_type->value.data;
 263+ l = r->headers_in.content_type->value.len;
 264+ escape = 2 * ngx_escape_uri(NULL, ct, l, NGX_ESCAPE_URI);
 265+
 266+ v->data = ngx_pnalloc(r->pool, l + escape);
 267+ if (v->data == NULL) {
 268+ return NGX_ERROR;
 269+ }
 270+
 271+ ngx_escape_uri(v->data, ct, l, NGX_ESCAPE_URI);
 272+
 273+ v->len = l + escape;
 274+ v->valid = 1;
 275+ v->no_cacheable = 0;
 276+ v->not_found = 0;
 277+
 278+ return NGX_OK;
 279+}
 280+
 281+static ngx_int_t
246282 ngx_http_udplog_add_variables(ngx_conf_t *cf)
247283 {
248284 ngx_http_variable_t *var, *v;

Comments

#Comment by Ottomata (talk | contribs)   21:01, 23 February 2012

Looks like it'll work, and is probably a very safe change.

I have to say though, ngx_http_udplog_escaped_content_type_variable() is an exact copy/paste of ngx_http_udplog_escaped_user_agent_variable(), except with one variable name changed and the use of r->headers_in.content_type instead of r->headers_in.user_agent. Ideaaaaaally, this logic would be DRYed up, but you know, this is C and a quick change to make this work, and DRYing it might break other unknown things.

I'd add some comments into the new function explaining that you know is is copy/pasted and that ideally it would be DRYed, just so that some future dev doesn't come in and say "AGH why didn't this guys DRY this up!?"

Anyway, looks good to me.

Status & tagging log