[patch][bugfix]Http mp4: replace strtod() with improved ngx_atofp() because precision problem.
胡聪 (hucc)
hucong.c at foxmail.com
Wed Oct 26 09:47:06 UTC 2016
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.
Sorry, my mistake, last patch lost some codes when i transfer codes
from Git to Mercurial.
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 1477473854 -28800
# Wed Oct 26 17:24:14 2016 +0800
# Node ID 652843788cb08c18dcbc6bab2857a228093767e4
# 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 652843788cb0 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 17:24:14 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 652843788cb0 src/core/ngx_string.h
--- a/src/core/ngx_string.h Fri Oct 21 16:28:39 2016 +0300
+++ b/src/core/ngx_string.h Wed Oct 26 17:24:14 2016 +0800
@@ -171,6 +171,7 @@
ngx_int_t ngx_atoi(u_char *line, size_t n);
ngx_int_t ngx_atofp(u_char *line, size_t n, size_t point);
+ngx_int_t ngx_atosp(u_char *line, size_t n, size_t point, unsigned strict);
ssize_t ngx_atosz(u_char *line, size_t n);
off_t ngx_atoof(u_char *line, size_t n);
time_t ngx_atotm(u_char *line, size_t n);
diff -r 56d6bfe6b609 -r 652843788cb0 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 17:24:14 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/742a8582/attachment.html>
More information about the nginx-devel
mailing list