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?
#lround(strtod())
--- a/src/http/modules/ngx_http_mp4_module.c
+++ b/src/http/modules/ngx_http_mp4_module.c
@@ -542,7 +542,7 @@ ngx_http_mp4_handler(ngx_http_request_t *r)
*/
ngx_set_errno(0);
- start = (int) (strtod((char *) value.data, NULL) * 1000);
+ start = (int) (lround(strtod((char *) value.data, NULL) * 1000));
if (ngx_errno != 0) {
start = -1;
@@ -552,7 +552,7 @@ ngx_http_mp4_handler(ngx_http_request_t *r)
if (ngx_http_arg(r, (u_char *) "end", 3, &value) == NGX_OK) {
ngx_set_errno(0);
- end = (int) (strtod((char *) value.data, NULL) * 1000);
+ end = (int) (lround(strtod((char *) value.data, NULL) * 1000));
#new function
+ngx_int_t
+ngx_atopi(u_char *line, size_t n, size_t point)
+{
+ 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;
+ }
+
+ dot = 1;
+ continue;
+ }
+
+ if (*line < '0' || *line > '9') {
+ return NGX_ERROR;
+ }
+
+ if (dot && point == 0) {
+ 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;
+}
------------------ Original ------------------
From: "Maxim Dounin";<mdounin@mdounin.ru>;
Send time: Wednesday, Oct 19, 2016 2:25 AM
To: "nginx-devel"<nginx-devel@nginx.org>;
Subject: Re: [patch][bugfix]Http mp4: replace strtod() with improved ngx_atofp() because precision problem.
Hello!
On Thu, Oct 13, 2016 at 04:45:23PM +0800, 月耳 wrote:
> # HG changeset patch
> # User hucongcong <hucong.c@foxmail.com>
> # Date 1476342771 -28800
> # Thu Oct 13 15:12:51 2016 +0800
> # Node ID 93bad8b82169245db6d4fe4e6b6c823221ee6a38
> # Parent 7cdf69d012f02a80c94d930b29c71847e203e4d6
> Http mp4: replace strtod() with improved ngx_atofp() because precision problem.
>
> function prototype is ngx_atofp(u_char *line, size_t n, size_t point).
> use case based on the old version:
> ngx_atofp("12.2193", 7, 0) returns NGX_ERROR,
> ngx_atofp("12.2193", 7, 2) returns NGX_ERROR.
> now, allow point = 0 or point less than the number of fractional digits.
> ngx_atofp("12.2193", 7, 0) returns 12,
> ngx_atofp("12.2193", 7, 2) returns 1221.
> retained all the required or right feature at the same time, e.g.,
> ngx_atofp("10.5", 4, 2) returns 1050.
>
> deprecated strtod() in ngx_http_mp4_module, since the precision problem,
> which was metioned in http://stackoverflow.com/questions/18361261, e.g.,
> (int) (strtod((char *) "32.480", NULL) * 1000) returns 32479.
> another way to solve this problem is like this round(strtod()), e.g.,
> (int) round((strtod((char *) "32.480", NULL) * 1000)) returns 32480,
> but its not necessary, since we have a better ngx_atofp().
>
> diff -r 7cdf69d012f0 -r 93bad8b82169 src/core/ngx_string.c
> --- a/src/core/ngx_string.c Tue Oct 11 18:03:01 2016 +0300
> +++ b/src/core/ngx_string.c Thu Oct 13 15:12:51 2016 +0800
> @@ -945,8 +945,8 @@
>
> for (value = 0; n--; line++) {
>
> - if (point == 0) {
> - return NGX_ERROR;
> + if (dot && point == 0) {
> + break;
> }
>
> if (*line == '.') {
With such a change it won't be possible to detect errors in other
places where ngx_atofp() is used. In particular, something like
split_clients $remote_addr $foo {
0.015% 1;
* 2;
}
will silently discard trailing 5. Moreover, even something like
split_clients $remote_addr $foo {
0.01foobar% 1;
* 2;
}
will be silently accepted.
[...]
--
Maxim Dounin
http://nginx.org/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
