Re: [FFmpeg-devel] [PATCH 3/4] ffserver_config: improve error handling
On 2 November 2014 00:02, Reynaldo H. Verdejo Pinochet reyna...@osg.samsung.com wrote: On 11/01/2014 07:59 PM, Lukasz Marek wrote: [..] I decreased it by 1 automatically rather than for any reason. I didn't want to change logic where it was not needed, and it was not needed here. I guess you ask in general if there should be a limit (not just it is 65535 or 6). I don't know, but as I stated before, I don't want to change the limit, just int parsing routine. If you wish I can change back to 65536 or to INT_MAX, but I want it to be clear it is not my original intention. :) Fair enough. We can work on those limits latter on. OK, will do that locally. Thanks. Feel free to push afterward. Pushed ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/4] ffserver_config: improve error handling
Hi On 10/31/2014 11:00 PM, Lukasz Marek wrote: [..] @@ -356,9 +356,7 @@ static int ffserver_parse_config_global(FFServerConfig *config, const char *cmd, if (!av_strcasecmp(cmd, Port)) WARNING(Port option is deprecated, use HTTPPort instead\n); ffserver_get_arg(arg, sizeof(arg), p); -val = atoi(arg); -if (val 1 || val 65536) -ERROR(Invalid port: %s\n, arg); +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid port: %s\n, arg); if (val 1024) WARNING(Trying to use IETF assigned system port: %d\n, val); config-http_addr.sin_port = htons(val); @@ -367,37 +365,38 @@ static int ffserver_parse_config_global(FFServerConfig *config, const char *cmd, WARNING(BindAddress option is deprecated, use HTTPBindAddress instead\n); ffserver_get_arg(arg, sizeof(arg), p); if (resolve_host(config-http_addr.sin_addr, arg) != 0) -ERROR(%s:%d: Invalid host/IP address: %s\n, arg); +ERROR(Invalid host/IP address: %s\n, arg); } else if (!av_strcasecmp(cmd, NoDaemon)) { WARNING(NoDaemon option has no effect, you should remove it\n); } else if (!av_strcasecmp(cmd, RTSPPort)) { ffserver_get_arg(arg, sizeof(arg), p); -val = atoi(arg); -if (val 1 || val 65536) -ERROR(%s:%d: Invalid port: %s\n, arg); -config-rtsp_addr.sin_port = htons(atoi(arg)); +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid port: %s\n, arg); +config-rtsp_addr.sin_port = htons(val); } else if (!av_strcasecmp(cmd, RTSPBindAddress)) { ffserver_get_arg(arg, sizeof(arg), p); if (resolve_host(config-rtsp_addr.sin_addr, arg) != 0) ERROR(Invalid host/IP address: %s\n, arg); } else if (!av_strcasecmp(cmd, MaxHTTPConnections)) { ffserver_get_arg(arg, sizeof(arg), p); -val = atoi(arg); -if (val 1 || val 65536) -ERROR(Invalid MaxHTTPConnections: %s\n, arg); +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid MaxHTTPConnections: %s\n, arg); I don't think we should be capping MaxHTTPConnections at 65535. If there's a reason then FFServerConfig.nb_max_http_connections type needs to be revised too? config-nb_max_http_connections = val; +if (config-nb_max_connections config-nb_max_http_connections) +ERROR(Inconsistent configuration: MaxClients(%d) MaxHTTPConnections(%d)\n, + config-nb_max_connections, config-nb_max_http_connections); } else if (!av_strcasecmp(cmd, MaxClients)) { ffserver_get_arg(arg, sizeof(arg), p); -val = atoi(arg); -if (val 1 || val config-nb_max_http_connections) -ERROR(Invalid MaxClients: %s\n, arg); -else -config-nb_max_connections = val; +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid MaxClients: %s\n, arg); +config-nb_max_connections = val; +if (config-nb_max_connections config-nb_max_http_connections) +ERROR(Inconsistent configuration: MaxClients(%d) MaxHTTPConnections(%d)\n, + config-nb_max_connections, config-nb_max_http_connections); Same as above [...] @@ -500,6 +499,9 @@ static int ffserver_parse_config_feed(FFServerConfig *config, const char *cmd, c case 'G': fsize *= 1024 * 1024 * 1024; break; +default: +ERROR(Invalid file size: %s\n, arg); +break; } feed-feed_max_size = (int64_t)fsize; if (feed-feed_max_size FFM_PACKET_SIZE*4) @@ -876,11 +878,15 @@ static int ffserver_parse_config_stream(FFServerConfig *config, const char *cmd, stream-is_multicast = 1; stream-loop = 1; /* default is looping */ } else if (!av_strcasecmp(cmd, MulticastPort)) { +int val; ffserver_get_arg(arg, sizeof(arg), p); -stream-multicast_port = atoi(arg); +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid MulticastPort: %s\n, arg); +stream-multicast_port = val; } else if (!av_strcasecmp(cmd, MulticastTTL)) { +int val; Maybe declare val once at the beginning of ffserver_parse_config_stream() Rest looks OK. Bests, -- Reynaldo H. Verdejo Pinochet Open Source Group Samsung Research America / Silicon Valley ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/4] ffserver_config: improve error handling
On 01.11.2014 23:06, Reynaldo H. Verdejo Pinochet wrote: Hi On 10/31/2014 11:00 PM, Lukasz Marek wrote: [..] @@ -356,9 +356,7 @@ static int ffserver_parse_config_global(FFServerConfig *config, const char *cmd, if (!av_strcasecmp(cmd, Port)) WARNING(Port option is deprecated, use HTTPPort instead\n); ffserver_get_arg(arg, sizeof(arg), p); -val = atoi(arg); -if (val 1 || val 65536) -ERROR(Invalid port: %s\n, arg); +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid port: %s\n, arg); if (val 1024) WARNING(Trying to use IETF assigned system port: %d\n, val); config-http_addr.sin_port = htons(val); @@ -367,37 +365,38 @@ static int ffserver_parse_config_global(FFServerConfig *config, const char *cmd, WARNING(BindAddress option is deprecated, use HTTPBindAddress instead\n); ffserver_get_arg(arg, sizeof(arg), p); if (resolve_host(config-http_addr.sin_addr, arg) != 0) -ERROR(%s:%d: Invalid host/IP address: %s\n, arg); +ERROR(Invalid host/IP address: %s\n, arg); } else if (!av_strcasecmp(cmd, NoDaemon)) { WARNING(NoDaemon option has no effect, you should remove it\n); } else if (!av_strcasecmp(cmd, RTSPPort)) { ffserver_get_arg(arg, sizeof(arg), p); -val = atoi(arg); -if (val 1 || val 65536) -ERROR(%s:%d: Invalid port: %s\n, arg); -config-rtsp_addr.sin_port = htons(atoi(arg)); +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid port: %s\n, arg); +config-rtsp_addr.sin_port = htons(val); } else if (!av_strcasecmp(cmd, RTSPBindAddress)) { ffserver_get_arg(arg, sizeof(arg), p); if (resolve_host(config-rtsp_addr.sin_addr, arg) != 0) ERROR(Invalid host/IP address: %s\n, arg); } else if (!av_strcasecmp(cmd, MaxHTTPConnections)) { ffserver_get_arg(arg, sizeof(arg), p); -val = atoi(arg); -if (val 1 || val 65536) -ERROR(Invalid MaxHTTPConnections: %s\n, arg); +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid MaxHTTPConnections: %s\n, arg); I don't think we should be capping MaxHTTPConnections at 65535. If there's a reason then FFServerConfig.nb_max_http_connections type needs to be revised too? I decreased it by 1 automatically rather than for any reason. I didn't want to change logic where it was not needed, and it was not needed here. I guess you ask in general if there should be a limit (not just it is 65535 or 6). I don't know, but as I stated before, I don't want to change the limit, just int parsing routine. If you wish I can change back to 65536 or to INT_MAX, but I want it to be clear it is not my original intention. :) [...] @@ -500,6 +499,9 @@ static int ffserver_parse_config_feed(FFServerConfig *config, const char *cmd, c case 'G': fsize *= 1024 * 1024 * 1024; break; +default: +ERROR(Invalid file size: %s\n, arg); +break; } feed-feed_max_size = (int64_t)fsize; if (feed-feed_max_size FFM_PACKET_SIZE*4) @@ -876,11 +878,15 @@ static int ffserver_parse_config_stream(FFServerConfig *config, const char *cmd, stream-is_multicast = 1; stream-loop = 1; /* default is looping */ } else if (!av_strcasecmp(cmd, MulticastPort)) { +int val; ffserver_get_arg(arg, sizeof(arg), p); -stream-multicast_port = atoi(arg); +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid MulticastPort: %s\n, arg); +stream-multicast_port = val; } else if (!av_strcasecmp(cmd, MulticastTTL)) { +int val; Maybe declare val once at the beginning of ffserver_parse_config_stream() OK, will do that locally. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/4] ffserver_config: improve error handling
On 11/01/2014 07:59 PM, Lukasz Marek wrote: [..] I decreased it by 1 automatically rather than for any reason. I didn't want to change logic where it was not needed, and it was not needed here. I guess you ask in general if there should be a limit (not just it is 65535 or 6). I don't know, but as I stated before, I don't want to change the limit, just int parsing routine. If you wish I can change back to 65536 or to INT_MAX, but I want it to be clear it is not my original intention. :) Fair enough. We can work on those limits latter on. OK, will do that locally. Thanks. Feel free to push afterward. Bests, -- Reynaldo H. Verdejo Pinochet Open Source Group Samsung Research America / Silicon Valley ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/4] ffserver_config: improve error handling
Replace atoi with more advanced parsing routine. Set maximum port value to 65535 (not 65536). Other checks. Signed-off-by: Lukasz Marek lukasz.m.lu...@gmail.com --- ffserver_config.c | 48 +++- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/ffserver_config.c b/ffserver_config.c index 6989bd8..c694934 100644 --- a/ffserver_config.c +++ b/ffserver_config.c @@ -356,9 +356,7 @@ static int ffserver_parse_config_global(FFServerConfig *config, const char *cmd, if (!av_strcasecmp(cmd, Port)) WARNING(Port option is deprecated, use HTTPPort instead\n); ffserver_get_arg(arg, sizeof(arg), p); -val = atoi(arg); -if (val 1 || val 65536) -ERROR(Invalid port: %s\n, arg); +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid port: %s\n, arg); if (val 1024) WARNING(Trying to use IETF assigned system port: %d\n, val); config-http_addr.sin_port = htons(val); @@ -367,37 +365,38 @@ static int ffserver_parse_config_global(FFServerConfig *config, const char *cmd, WARNING(BindAddress option is deprecated, use HTTPBindAddress instead\n); ffserver_get_arg(arg, sizeof(arg), p); if (resolve_host(config-http_addr.sin_addr, arg) != 0) -ERROR(%s:%d: Invalid host/IP address: %s\n, arg); +ERROR(Invalid host/IP address: %s\n, arg); } else if (!av_strcasecmp(cmd, NoDaemon)) { WARNING(NoDaemon option has no effect, you should remove it\n); } else if (!av_strcasecmp(cmd, RTSPPort)) { ffserver_get_arg(arg, sizeof(arg), p); -val = atoi(arg); -if (val 1 || val 65536) -ERROR(%s:%d: Invalid port: %s\n, arg); -config-rtsp_addr.sin_port = htons(atoi(arg)); +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid port: %s\n, arg); +config-rtsp_addr.sin_port = htons(val); } else if (!av_strcasecmp(cmd, RTSPBindAddress)) { ffserver_get_arg(arg, sizeof(arg), p); if (resolve_host(config-rtsp_addr.sin_addr, arg) != 0) ERROR(Invalid host/IP address: %s\n, arg); } else if (!av_strcasecmp(cmd, MaxHTTPConnections)) { ffserver_get_arg(arg, sizeof(arg), p); -val = atoi(arg); -if (val 1 || val 65536) -ERROR(Invalid MaxHTTPConnections: %s\n, arg); +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid MaxHTTPConnections: %s\n, arg); config-nb_max_http_connections = val; +if (config-nb_max_connections config-nb_max_http_connections) +ERROR(Inconsistent configuration: MaxClients(%d) MaxHTTPConnections(%d)\n, + config-nb_max_connections, config-nb_max_http_connections); } else if (!av_strcasecmp(cmd, MaxClients)) { ffserver_get_arg(arg, sizeof(arg), p); -val = atoi(arg); -if (val 1 || val config-nb_max_http_connections) -ERROR(Invalid MaxClients: %s\n, arg); -else -config-nb_max_connections = val; +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, Invalid MaxClients: %s\n, arg); +config-nb_max_connections = val; +if (config-nb_max_connections config-nb_max_http_connections) +ERROR(Inconsistent configuration: MaxClients(%d) MaxHTTPConnections(%d)\n, + config-nb_max_connections, config-nb_max_http_connections); } else if (!av_strcasecmp(cmd, MaxBandwidth)) { int64_t llval; +char *tailp; ffserver_get_arg(arg, sizeof(arg), p); -llval = strtoll(arg, NULL, 10); -if (llval 10 || llval 1000) +errno = 0; +llval = strtoll(arg, tailp, 10); +if (llval 10 || llval 1000 || tailp[0] || errno) ERROR(Invalid MaxBandwidth: %s\n, arg); else config-max_bandwidth = llval; @@ -405,7 +404,7 @@ static int ffserver_parse_config_global(FFServerConfig *config, const char *cmd, if (!config-debug) ffserver_get_arg(config-logfilename, sizeof(config-logfilename), p); } else if (!av_strcasecmp(cmd, LoadModule)) { -ERROR(Loadable modules no longer supported\n); +ERROR(Loadable modules are no longer supported\n); } else ERROR(Incorrect keyword: '%s'\n, cmd); return 0; @@ -500,6 +499,9 @@ static int ffserver_parse_config_feed(FFServerConfig *config, const char *cmd, c case 'G': fsize *= 1024 * 1024 * 1024; break; +default: +ERROR(Invalid file size: %s\n, arg); +break; } feed-feed_max_size = (int64_t)fsize; if (feed-feed_max_size FFM_PACKET_SIZE*4) @@ -876,11 +878,15 @@ static int ffserver_parse_config_stream(FFServerConfig *config, const char *cmd, stream-is_multicast = 1; stream-loop = 1;