Hello!

On Wed, Oct 19, 2016 at 02:43:24PM +0800, 胡聪 (hucc) wrote:

> Hello!
>
>
> Thanks for reminding me! Illegal character should be checked
> also error detection in configration parser is needed.
>
>
> Considering that ngx_atofp() was used in some 3rd party modules,
> Would nginx-devel accept a new function like ngx_ato??() or
> lround(strtod()), or just let strtod() alone?

Using strtod() should be avoided as it has other problems, see
https://trac.nginx.org/nginx/ticket/475. That is, an additional
variant of ngx_atofp() is a way to go.

Last time we've looked into this, we've stumbled upon selecting a
proper name for the additional function. Looking into this again
I tend to think that proper solution would be to use a special
function withing the ngx_http_mp4_module itself. Patch below.

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1477416871 -10800
# Tue Oct 25 20:34:31 2016 +0300
# Node ID 16dae7d3b770c63dc16ac06312a66c1bafc3308f
# Parent 56d6bfe6b609c565a9f500bde573fd9b488ff960
Mp4: introduced custom version of ngx_atofp().

This allows to correctly parse "start" and "end" arguments without
null-termination (ticket #475), and also fixes rounding errors observed
with strtod() when using i387 instructions.

diff --git a/src/http/modules/ngx_http_mp4_module.c b/src/http/modules/ngx_http_mp4_module.c
--- a/src/http/modules/ngx_http_mp4_module.c
+++ b/src/http/modules/ngx_http_mp4_module.c
@@ -216,6 +216,7 @@ typedef struct {


static ngx_int_t ngx_http_mp4_handler(ngx_http_request_t *r);
+static ngx_int_t ngx_http_mp4_atofp(u_char *line, size_t n, size_t point);

static ngx_int_t ngx_http_mp4_process(ngx_http_mp4_file_t *mp4);
static ngx_int_t ngx_http_mp4_read_atom(ngx_http_mp4_file_t *mp4,
@@ -537,26 +538,15 @@ ngx_http_mp4_handler(ngx_http_request_t

/*
* A Flash player may send start value with a lot of digits
- * after dot so strtod() is used instead of atofp(). NaNs and
- * infinities become negative numbers after (int) conversion.
+ * after dot so a custom function is used instead of ngx_atofp().
*/

- ngx_set_errno(0);
- start = (int) (strtod((char *) value.data, NULL) * 1000);
-
- if (ngx_errno != 0) {
- start = -1;
- }
+ start = ngx_http_mp4_atofp(value.data, value.len, 3);
}

if (ngx_http_arg(r, (u_char *) "end", 3, &value) == NGX_OK) {

- ngx_set_errno(0);
- end = (int) (strtod((char *) value.data, NULL) * 1000);
-
- if (ngx_errno != 0) {
- end = -1;
- }
+ end = ngx_http_mp4_atofp(value.data, value.len, 3);

if (end > 0) {
if (start < 0) {
@@ -687,6 +677,62 @@ ngx_http_mp4_handler(ngx_http_request_t


static ngx_int_t
+ngx_http_mp4_atofp(u_char *line, size_t n, size_t point)
+{
+ ngx_int_t value, cutoff, cutlim;
+ ngx_uint_t dot;
+
+ /* same as ngx_atofp(), but allows additional digits */
+
+ if (n == 0) {
+ return NGX_ERROR;
+ }
+
+ cutoff = NGX_MAX_INT_T_VALUE / 10;
+ cutlim = NGX_MAX_INT_T_VALUE % 10;
+
+ dot = 0;
+
+ for (value = 0; n--; line++) {
+
+ if (point == 0) {
+ break;
+ }
+
+ if (*line == '.') {
+ if (dot) {
+ return NGX_ERROR;
+ }
+
+ dot = 1;
+ continue;
+ }
+
+ if (*line < '0' || *line > '9') {
+ return NGX_ERROR;
+ }
+
+ if (value >= cutoff && (value > cutoff || *line - '0' > cutlim)) {
+ return NGX_ERROR;
+ }
+
+ value = value * 10 + (*line - '0');
+ point -= dot;
+ }
+
+ while (point--) {
+ if (value > cutoff) {
+ return NGX_ERROR;
+ }
+
+ value = value * 10;
+ }
+
+ return value;
+}
+
+
+static ngx_int_t
ngx_http_mp4_process(ngx_http_mp4_file_t *mp4)
{
off_t start_offset, end_offset, adjustment;

--
Maxim Dounin
http://nginx.org/

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel