<div>Hello!</div><div><br></div><div>Thanks for reminding me! Illegal character should be checked</div><div>also error detection in configration parser is needed.</div><div><br></div><div>Considering that ngx_atofp() was used in some 3rd party modules,</div><div>Would nginx-devel accept a new function like ngx_ato??() or</div><div>lround(strtod()), or just let strtod() alone?</div><div><div><br></div><div>#lround(strtod())</div><div>
<style type="text/css">p.p1 {margin: 0.0px 0.0px 0.0px 0.0px; font: 13.0px Menlo; color: #000000; background-color: #ffffff}
p.p2 {margin: 0.0px 0.0px 0.0px 0.0px; font: 13.0px Menlo; color: #000000; background-color: #ffffff; min-height: 15.0px}
p.p3 {margin: 0.0px 0.0px 0.0px 0.0px; font: 13.0px Menlo; color: #c33720; background-color: #ffffff}
p.p4 {margin: 0.0px 0.0px 0.0px 0.0px; font: 13.0px Menlo; color: #34bc26; background-color: #ffffff}
span.s1 {font-variant-ligatures: no-common-ligatures}
span.s2 {font-variant-ligatures: no-common-ligatures; color: #33bbc8}
</style>
<p class="p1"><span class="s1"><b>--- a/src/http/modules/ngx_http_mp4_module.c</b></span></p>
<p class="p1"><span class="s1"><b>+++ b/src/http/modules/ngx_http_mp4_module.c</b></span></p>
<p class="p1"><span class="s2">@@ -542,7 +542,7 @@</span><span class="s1"> ngx_http_mp4_handler(ngx_http_request_t *r)</span></p>
<p class="p1"><span class="s1"> */</span></p>
<p class="p2"><span class="s1"> </span></p>
<p class="p1"><span class="s1"> ngx_set_errno(0);</span></p>
<p class="p3"><span class="s1">- start = (int) (strtod((char *) value.data, NULL) * 1000);</span></p>
<p class="p4"><span class="s1">+ start = (int) (lround(strtod((char *) value.data, NULL) * 1000));</span></p>
<p class="p2"><span class="s1"> </span></p>
<p class="p1"><span class="s1"> if (ngx_errno != 0) {</span></p>
<p class="p1"><span class="s1"> start = -1;</span></p>
<p class="p1"><span class="s2">@@ -552,7 +552,7 @@</span><span class="s1"> ngx_http_mp4_handler(ngx_http_request_t *r)</span></p>
<p class="p1"><span class="s1"> if (ngx_http_arg(r, (u_char *) "end", 3, &value) == NGX_OK) {</span></p>
<p class="p2"><span class="s1"> </span></p>
<p class="p1"><span class="s1"> ngx_set_errno(0);</span></p>
<p class="p3"><span class="s1">- end = (int) (strtod((char *) value.data, NULL) * 1000);</span></p>
<p class="p4"><span class="s1">+ end = (int) (lround(strtod((char *) value.data, NULL) * 1000));</span></p></div><div><br></div><div>#new function</div><div>
<style type="text/css">p.p1 {margin: 0.0px 0.0px 0.0px 0.0px; font: 13.0px Menlo; color: #34bc26; background-color: #ffffff}
span.s1 {font-variant-ligatures: no-common-ligatures}
</style>
<p class="p1"><span class="s1">+ngx_int_t</span></p>
<p class="p1"><span class="s1">+ngx_atopi(u_char *line, size_t n, size_t point)</span></p>
<p class="p1"><span class="s1">+{</span></p>
<p class="p1"><span class="s1">+ ngx_int_t value, cutoff, cutlim;</span></p>
<p class="p1"><span class="s1">+ ngx_uint_t dot;</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ if (n == 0) {</span></p>
<p class="p1"><span class="s1">+ return NGX_ERROR;</span></p>
<p class="p1"><span class="s1">+ }</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ cutoff = NGX_MAX_INT_T_VALUE / 10;</span></p>
<p class="p1"><span class="s1">+ cutlim = NGX_MAX_INT_T_VALUE % 10;</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ dot = 0;</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ for (value = 0; n--; line++) {</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ if (*line == '.') {</span></p>
<p class="p1"><span class="s1">+ if (dot) {</span></p>
<p class="p1"><span class="s1">+ return NGX_ERROR;</span></p>
<p class="p1"><span class="s1">+ }</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ dot = 1;</span></p>
<p class="p1"><span class="s1">+ continue;</span></p>
<p class="p1"><span class="s1">+ }</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ if (*line < '0' || *line > '9') {</span></p>
<p class="p1"><span class="s1">+ return NGX_ERROR;</span></p>
<p class="p1"><span class="s1">+ }</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ if (dot && point == 0) {</span></p>
<p class="p1"><span class="s1">+ continue;</span></p>
<p class="p1"><span class="s1">+ }</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ if (value >= cutoff && (value > cutoff || *line - '0' > cutlim)) {</span></p>
<p class="p1"><span class="s1">+ return NGX_ERROR;</span></p>
<p class="p1"><span class="s1">+ }</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ value = value * 10 + (*line - '0');</span></p>
<p class="p1"><span class="s1">+ point -= dot;</span></p>
<p class="p1"><span class="s1">+ }</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ while (point--) {</span></p>
<p class="p1"><span class="s1">+ if (value > cutoff) {</span></p>
<p class="p1"><span class="s1">+ return NGX_ERROR;</span></p>
<p class="p1"><span class="s1">+ }</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ value = value * 10;</span></p>
<p class="p1"><span class="s1">+ }</span></p>
<p class="p1"><span class="s1">+</span></p>
<p class="p1"><span class="s1">+ return value;</span></p>
<p class="p1"><span class="s1">+}</span></p></div><div style="font-size: 12px;font-family: Arial Narrow;padding:2px 0 2px 0;">------------------ Original ------------------</div><div style="font-size: 12px;background:#efefef;padding:8px;"><div><b>From: </b> "Maxim Dounin";<mdounin@mdounin.ru>;</div><div><b>Send time:</b> Wednesday, Oct 19, 2016 2:25 AM</div><div><b>To:</b> "nginx-devel"<nginx-devel@nginx.org>; <wbr></div><div></div><div><b>Subject: </b> Re: [patch][bugfix]Http mp4: replace strtod() with improved ngx_atofp() because precision problem.</div></div><div><br></div>Hello!<br><br>On Thu, Oct 13, 2016 at 04:45:23PM +0800, Ô¶ú wrote:<br><br>> # HG changeset patch<br>> # User hucongcong <hucong.c@foxmail.com><br>> # Date 1476342771 -28800<br>> # Thu Oct 13 15:12:51 2016 +0800<br>> # Node ID 93bad8b82169245db6d4fe4e6b6c823221ee6a38<br>> # Parent 7cdf69d012f02a80c94d930b29c71847e203e4d6<br>> Http mp4: replace strtod() with improved ngx_atofp() because precision problem.<br>> <br>> function prototype is ngx_atofp(u_char *line, size_t n, size_t point).<br>> use case based on the old version:<br>> ngx_atofp("12.2193", 7, 0) returns NGX_ERROR,<br>> ngx_atofp("12.2193", 7, 2) returns NGX_ERROR.<br>> now, allow point = 0 or point less than the number of fractional digits.<br>> ngx_atofp("12.2193", 7, 0) returns 12,<br>> ngx_atofp("12.2193", 7, 2) returns 1221.<br>> retained all the required or right feature at the same time, e.g.,<br>> ngx_atofp("10.5", 4, 2) returns 1050.<br>> <br>> deprecated strtod() in ngx_http_mp4_module, since the precision problem,<br>> which was metioned in http://stackoverflow.com/questions/18361261, e.g.,<br>> (int) (strtod((char *) "32.480", NULL) * 1000) returns 32479.<br>> another way to solve this problem is like this round(strtod()), e.g.,<br>> (int) round((strtod((char *) "32.480", NULL) * 1000)) returns 32480,<br>> but its not necessary, since we have a better ngx_atofp().<br>> <br>> diff -r 7cdf69d012f0 -r 93bad8b82169 src/core/ngx_string.c<br>> --- a/src/core/ngx_string.c Tue Oct 11 18:03:01 2016 +0300<br>> +++ b/src/core/ngx_string.c Thu Oct 13 15:12:51 2016 +0800<br>> @@ -945,8 +945,8 @@<br>> <br>> for (value = 0; n--; line++) {<br>> <br>> - if (point == 0) {<br>> - return NGX_ERROR;<br>> + if (dot && point == 0) {<br>> + break;<br>> }<br>> <br>> if (*line == '.') {</div><div><br>With such a change it won't be possible to detect errors in other <br>places where ngx_atofp() is used. In particular, something like<br><br> split_clients $remote_addr $foo {<br> 0.015% 1;<br> * 2;<br> }<br><br>will silently discard trailing 5. Moreover, even something like<br><br> split_clients $remote_addr $foo {<br> 0.01foobar% 1;<br> * 2;<br> }<br><br>will be silently accepted.<br><br>[...]<br><br>-- <br>Maxim Dounin<br>http://nginx.org/<br><br>_______________________________________________<br>nginx-devel mailing list<br>nginx-devel@nginx.org<br>http://mailman.nginx.org/mailman/listinfo/nginx-devel</div>