Welcome! Log In Create A New Profile

Advanced

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

胡聪 (hucc)
October 19, 2016 02:46AM
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
Subject Author Views Posted

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

月耳 413 October 13, 2016 04:48AM

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

Maxim Dounin 148 October 18, 2016 02:28PM

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

胡聪 (hucc) 172 October 19, 2016 02:46AM

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

Maxim Dounin 237 October 25, 2016 03:36PM

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

胡聪 (hucc) 141 October 26, 2016 05:12AM

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

Maxim Dounin 144 October 26, 2016 08:32AM

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

胡聪 (hucc) 155 October 26, 2016 05:48AM

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

胡聪 (hucc) 143 October 26, 2016 12:42PM

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

Maxim Dounin 168 October 26, 2016 01:58PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 53
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready