You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2011/04/03 15:18:30 UTC
Re: svn commit: r1086756 - in /httpd/httpd/trunk: docs/manual/developer/new_api_2_4.xml
include/ap_mmn.h include/http_config.h modules/lua/mod_lua.c server/config.c
server/util.c
On 03/29/2011 11:29 PM, sf@apache.org wrote:
> Author: sf
> Date: Tue Mar 29 21:29:34 2011
> New Revision: 1086756
>
> URL: http://svn.apache.org/viewvc?rev=1086756&view=rev
> Log:
> Change the ap_cfg_getline() and ap_cfg_getc() to return an error code.
>
> Also:
> - Make ap_cfg_getline() return APR_ENOSPC if a config line is too long.
> - Add ap_pcfg_strerror() function to convert ap_cfg_getline's return value
> into a nice message.
> - Adjust definition of ap_configfile_t accordingly.
>
> Not bumping MMN because it has already been bumped today.
>
> Modified:
> httpd/httpd/trunk/docs/manual/developer/new_api_2_4.xml
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/http_config.h
> httpd/httpd/trunk/modules/lua/mod_lua.c
> httpd/httpd/trunk/server/config.c
> httpd/httpd/trunk/server/util.c
>
> Modified: httpd/httpd/trunk/server/util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1086756&r1=1086755&r2=1086756&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/util.c (original)
> +++ httpd/httpd/trunk/server/util.c Tue Mar 29 21:29:34 2011
> @@ -874,170 +864,156 @@ AP_DECLARE(apr_status_t) ap_pcfg_openfil
>
>
> /* Allocate a ap_configfile_t handle with user defined functions and params */
> -AP_DECLARE(ap_configfile_t *) ap_pcfg_open_custom(apr_pool_t *p,
> - const char *descr,
> - void *param,
> - int(*getch)(void *param),
> - void *(*getstr) (void *buf, size_t bufsiz, void *param),
> - int(*close_func)(void *param))
> +AP_DECLARE(ap_configfile_t *) ap_pcfg_open_custom(
> + apr_pool_t *p, const char *descr, void *param,
> + apr_status_t (*getc_func) (char *ch, void *param),
> + apr_status_t (*gets_func) (void *buf, size_t bufsize, void *param),
> + apr_status_t (*close_func) (void *param))
> {
> ap_configfile_t *new_cfg = apr_palloc(p, sizeof(*new_cfg));
> -#ifdef DEBUG
> - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL,
> - "Opening config handler %s", descr);
> -#endif
> new_cfg->param = param;
> new_cfg->name = descr;
> - new_cfg->getch = getch;
> - new_cfg->getstr = getstr;
> + new_cfg->getch = getc_func;
> + new_cfg->getstr = gets_func;
> new_cfg->close = close_func;
> new_cfg->line_number = 0;
> return new_cfg;
> }
>
> /* Read one character from a configfile_t */
> -AP_DECLARE(int) ap_cfg_getc(ap_configfile_t *cfp)
> +AP_DECLARE(apr_status_t) ap_cfg_getc(char *ch, ap_configfile_t *cfp)
> {
> - register int ch = cfp->getch(cfp->param);
> - if (ch == LF)
> + apr_status_t rc = cfp->getch(ch, cfp->param);
> + if (rc == APR_SUCCESS && *ch == LF)
> ++cfp->line_number;
> - return ch;
> + return rc;
> +}
> +
> +AP_DECLARE(const char *) ap_pcfg_strerror(apr_pool_t *p, ap_configfile_t *cfp,
> + apr_status_t rc)
> +{
> + char buf[MAX_STRING_LEN];
> + if (rc == APR_SUCCESS)
> + return NULL;
> + return apr_psprintf(p, "Error reading %s at line %d: %s",
> + cfp->name, cfp->line_number,
> + rc == APR_ENOSPC ? "Line too long"
> + : apr_strerror(rc, buf, sizeof(buf)));
> }
>
> /* Read one line from open ap_configfile_t, strip LF, increase line number */
> /* If custom handler does not define a getstr() function, read char by char */
> -AP_DECLARE(int) ap_cfg_getline(char *buf, size_t bufsize, ap_configfile_t *cfp)
> +AP_DECLARE(apr_status_t) ap_cfg_getline(char *buf, size_t bufsize, ap_configfile_t *cfp)
> {
> + apr_status_t rc;
> + char *src, *dst;
> /* If a "get string" function is defined, use it */
> if (cfp->getstr != NULL) {
> - char *src, *dst;
> char *cp;
> char *cbuf = buf;
> size_t cbufsize = bufsize;
>
> while (1) {
> ++cfp->line_number;
> - if (cfp->getstr(cbuf, cbufsize, cfp->param) == NULL)
> - return 1;
> + rc = cfp->getstr(cbuf, cbufsize, cfp->param);
> + if (rc == APR_EOF) {
> + if (cbuf != buf) {
> + *cbuf = '\0';
> + break;
> + }
> + else {
> + return APR_EOF;
> + }
> + }
> + if (rc != APR_SUCCESS) {
> + return rc;
> + }
>
> /*
> * check for line continuation,
> * i.e. match [^\\]\\[\r]\n only
> */
> cp = cbuf;
> - while (cp < cbuf+cbufsize && *cp != '\0')
> - cp++;
> + cp += strlen(cp);
> if (cp > cbuf && cp[-1] == LF) {
> cp--;
> if (cp > cbuf && cp[-1] == CR)
> cp--;
> if (cp > cbuf && cp[-1] == '\\') {
> cp--;
> - if (!(cp > cbuf && cp[-1] == '\\')) {
> - /*
> - * line continuation requested -
> - * then remove backslash and continue
> - */
> - cbufsize -= (cp-cbuf);
> - cbuf = cp;
> - continue;
Why don't we consider escaped backslashes as literals any longer?
> - }
> - else {
> - /*
> - * no real continuation because escaped -
> - * then just remove escape character
> - */
> - for ( ; cp < cbuf+cbufsize && *cp != '\0'; cp++)
> - cp[0] = cp[1];
> - }
> + /*
> + * line continuation requested -
> + * then remove backslash and continue
> + */
> + cbufsize -= (cp-cbuf);
> + cbuf = cp;
> + continue;
> }
> }
> + else if (cp - buf >= bufsize - 1) {
> + return APR_ENOSPC;
> + }
> break;
> }
> -
> - /*
> - * Leading and trailing white space is eliminated completely
> - */
> - src = buf;
> - while (apr_isspace(*src))
> - ++src;
> - /* blast trailing whitespace */
> - dst = &src[strlen(src)];
> - while (--dst >= src && apr_isspace(*dst))
> - *dst = '\0';
> - /* Zap leading whitespace by shifting */
> - if (src != buf)
> - for (dst = buf; (*dst++ = *src++) != '\0'; )
> - ;
> -
> -#ifdef DEBUG_CFG_LINES
> - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL, "Read config: %s", buf);
> -#endif
> - return 0;
> } else {
> /* No "get string" function defined; read character by character */
> - register int c;
> - register size_t i = 0;
> + size_t i = 0;
>
> - buf[0] = '\0';
> - /* skip leading whitespace */
> - do {
> - c = cfp->getch(cfp->param);
> - } while (c == '\t' || c == ' ');
> -
> - if (c == EOF)
> - return 1;
> -
> - if(bufsize < 2) {
> + if (bufsize < 2) {
> /* too small, assume caller is crazy */
> - return 1;
> + return APR_EINVAL;
> }
> + buf[0] = '\0';
>
> while (1) {
> - if ((c == '\t') || (c == ' ')) {
> - buf[i++] = ' ';
> - while ((c == '\t') || (c == ' '))
> - c = cfp->getch(cfp->param);
> - }
> - if (c == CR) {
> - /* silently ignore CR (_assume_ that a LF follows) */
> - c = cfp->getch(cfp->param);
Why don't we ignore CR any longer?
> + char c;
> + rc = cfp->getch(&c, cfp->param);
> + if (rc == APR_EOF) {
> + if (i > 0)
> + break;
> + else
> + return APR_EOF;
> }
> + if (rc != APR_SUCCESS)
> + return rc;
> if (c == LF) {
> - /* increase line number and return on LF */
> ++cfp->line_number;
> - }
> - if (c == EOF || c == 0x4 || c == LF || i >= (bufsize - 2)) {
> - /*
> - * check for line continuation
> - */
> + /* check for line continuation */
> if (i > 0 && buf[i-1] == '\\') {
> i--;
> - if (!(i > 0 && buf[i-1] == '\\')) {
> - /* line is continued */
> - c = cfp->getch(cfp->param);
> - continue;
> - }
> - /* else nothing needs be done because
> - * then the backslash is escaped and
> - * we just strip to a single one
> - */
> + continue;
> }
> - /* blast trailing whitespace */
> - while (i > 0 && apr_isspace(buf[i - 1]))
> - --i;
> - buf[i] = '\0';
> -#ifdef DEBUG_CFG_LINES
> - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL,
> - "Read config: %s", buf);
> -#endif
> - return 0;
> + else {
> + break;
> + }
> + }
> + else if (i >= bufsize - 2) {
> + return APR_ENOSPC;
> }
> buf[i] = c;
> ++i;
> - c = cfp->getch(cfp->param);
> }
> + buf[i] = '\0';
> }
> +
> + /*
> + * Leading and trailing white space is eliminated completely
> + */
> + src = buf;
> + while (apr_isspace(*src))
> + ++src;
> + /* blast trailing whitespace */
> + dst = &src[strlen(src)];
> + while (--dst >= src && apr_isspace(*dst))
> + *dst = '\0';
> + /* Zap leading whitespace by shifting */
> + if (src != buf)
> + memmove(buf, src, dst - src + 2);
> +
> +#ifdef DEBUG_CFG_LINES
> + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, "Read config: '%s'", buf);
> +#endif
> + return APR_SUCCESS;
> }
>
> /* Size an HTTP header field list item, as separated by a comma.
>
>
>
>
Regards
Rüdiger
Re: svn commit: r1086756 - in /httpd/httpd/trunk:
docs/manual/developer/new_api_2_4.xml include/ap_mmn.h include/http_config.h
modules/lua/mod_lua.c server/config.c server/util.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Sun, 3 Apr 2011, Ruediger Pluem wrote:
> On 03/29/2011 11:29 PM, sf@apache.org wrote:
>> Author: sf
>> Date: Tue Mar 29 21:29:34 2011
>> New Revision: 1086756
>>
>> URL: http://svn.apache.org/viewvc?rev=1086756&view=rev
>> Log:
>> Change the ap_cfg_getline() and ap_cfg_getc() to return an error code.
>>
>> Also:
>> - Make ap_cfg_getline() return APR_ENOSPC if a config line is too long.
>> - Add ap_pcfg_strerror() function to convert ap_cfg_getline's return value
>> into a nice message.
>> - Adjust definition of ap_configfile_t accordingly.
>>
>> Not bumping MMN because it has already been bumped today.
>> Modified: httpd/httpd/trunk/server/util.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1086756&r1=1086755&r2=1086756&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/server/util.c (original)
>> +++ httpd/httpd/trunk/server/util.c Tue Mar 29 21:29:34 2011
>> - if (!(cp > cbuf && cp[-1] == '\\')) {
>> - /*
>> - * line continuation requested -
>> - * then remove backslash and continue
>> - */
>> - cbufsize -= (cp-cbuf);
>> - cbuf = cp;
>> - continue;
>
> Why don't we consider escaped backslashes as literals any longer?
Because the code was wrong to begin with. To be correct, it would have to
treat "\\" at EOL differently from "\\\", but it didn't do that. And since
this feature was undocumented, I thought it was not worth the hassle. The
user can prevent line continuation by appending a space (and that's
documented).
>> - if (c == CR) {
>> - /* silently ignore CR (_assume_ that a LF follows) */
>> - c = cfp->getch(cfp->param);
>
> Why don't we ignore CR any longer?
Any trailing CR is later removed together with other trailing whitespace.
I don't think we need to care about non-trailing CRs.
>> +
>> + /*
>> + * Leading and trailing white space is eliminated completely
>> + */
>> + src = buf;
>> + while (apr_isspace(*src))
>> + ++src;
>> + /* blast trailing whitespace */
>> + dst = &src[strlen(src)];
>> + while (--dst >= src && apr_isspace(*dst))
>> + *dst = '\0';
>> + /* Zap leading whitespace by shifting */
>> + if (src != buf)
>> + memmove(buf, src, dst - src + 2);
Cheers,
Stefan