[patch][bugfix]Http mp4: replace strtod() with improved ngx_atofp() because precision problem.

[胡聪 (hucc)]

Hello!


On Wed, Oct 26, 2016 3:34 AM, Maxim Dounin wrote:

>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.


VOD (video on demand) module which support requesting time range
also need the special function, so i would like to see that nginx-devel
add a function in Core, that would be helpful. Possible patch below.


 
# HG changeset patch

# User hucongcong <hucong.c at foxmail.com>

# Date 1477468236 -28800

#      Wed Oct 26 15:50:36 2016 +0800

# Node ID 6376b8da3689011dfc8805381970f9ea2251c0a6

# Parent  56d6bfe6b609c565a9f500bde573fd9b488ff960

Core: add ngx_atosp().




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 -r 56d6bfe6b609 -r 6376b8da3689 src/core/ngx_string.c

--- a/src/core/ngx_string.c	Fri Oct 21 16:28:39 2016 +0300

+++ b/src/core/ngx_string.c	Wed Oct 26 15:50:36 2016 +0800

@@ -926,7 +926,9 @@

 }

 

 

-/* parse a fixed point number, e.g., ngx_atofp("10.5", 4, 2) returns 1050 */

+/* parse a fixed point number, e.g., ngx_atofp("10.5", 4, 2) returns 1050.

+ * can be replaced by ngx_atosp(line, n, point, 1).

+ */

 

 ngx_int_t

 ngx_atofp(u_char *line, size_t n, size_t point)

@@ -982,6 +984,76 @@

 }

 

 

+/* parse a fixed point number and do bounds check when strict is true, e.g.,

+ * ngx_atosp("12.2193", 3, 0, 0) returns 12,

+ * ngx_atosp("12.2193", 3, 0, 1) returns NGX_ERROR,

+ * ngx_atosp("12.2193", 6, 3, 1) returns 12219,

+ * ngx_atosp("12.2193", 7, 3, 1) returns NGX_ERROR,

+ * ngx_atosp("12.2193", 7, 3, 0) returns 12219.

+ */

+

+ngx_int_t

+ngx_atosp(u_char *line, size_t n, size_t point, unsigned strict)

+{

+    ngx_int_t   value, cutoff, cutlim;

+    ngx_uint_t  dot;

+

+    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 (*line == '.') {

+            if (dot) {

+                return NGX_ERROR;

+            }

+

+            if (strict && point == 0) {

+                return NGX_ERROR;

+            }

+

+            dot = 1;

+            continue;

+        }

+

+        if (*line < '0' || *line > '9') {

+            return NGX_ERROR;

+        }

+

+        if (dot && point == 0) {

+            if (strict) {

+                return NGX_ERROR;

+            }

+

+            continue;

+        }

+

+        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;

+}

+

+

 ssize_t

 ngx_atosz(u_char *line, size_t n)

 {

diff -r 56d6bfe6b609 -r 6376b8da3689 src/http/modules/ngx_http_mp4_module.c

--- a/src/http/modules/ngx_http_mp4_module.c	Fri Oct 21 16:28:39 2016 +0300

+++ b/src/http/modules/ngx_http_mp4_module.c	Wed Oct 26 15:50:36 2016 +0800

@@ -537,26 +537,15 @@

 

             /*

              * 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 ngx_atosp() 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_atosp(value.data, value.len, 3, 0);

         }

 

         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_atosp(value.data, value.len, 3, 0);

 

             if (end > 0) {

                 if (start < 0) {
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20161026/83217f78/attachment.html>