Bug 63607 - Inconsistent translation of error code in server/util
Summary: Inconsistent translation of error code in server/util
Status: NEW
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: Core (show other bugs)
Version: 2.5-HEAD
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-24 17:18 UTC by Error Reporter
Modified: 2019-07-24 17:18 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Error Reporter 2019-07-24 17:18:13 UTC
We find that in ap_parse_form_data() at server/util.c:

AP_DECLARE(int) ap_parse_form_data(request_rec *r, ap_filter_t *f,
                                   apr_array_header_t **ptr,
                                   apr_size_t num, apr_size_t usize)
{
...
2822: rv = apr_bucket_read(bucket, &data, &len, APR_BLOCK_READ);
            if (rv != APR_SUCCESS) {
                apr_brigade_destroy(bb);
                return HTTP_BAD_REQUEST;
            }
...
}

apr_bucket_read() is a macro to a function pointer read, and it can call a function file_bucket_read() which can return error code EINVAL from file_bucket_read() -> apr_file_seek()

This means EINVAL(22) is translated to a client-side error HTTP_BAD_REQUEST(400), which I think there may be wrong since the error code means there is something wrong with the server.

Let's look at another translation at mod_cache, modules/cache/mod_cache_socache.c:

static int socache_precfg(apr_pool_t *pconf, apr_pool_t *plog, apr_pool_t *ptmp)
{
...
    apr_status_t rv = ap_mutex_register(pconf, cache_socache_id, NULL,
            APR_LOCK_DEFAULT, 0);
    if (rv != APR_SUCCESS) {
        ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, plog, APLOGNO(02390)
                "failed to register %s mutex", cache_socache_id);
1415:        return 500; /* An HTTP status would be a misnomer! */
    }
...
}

Here ap_mutex_register() can return EINVAL as well, but it is correctly translated to server-side error 500. Maybe as the comment suggests, it should not be translated to an HTTP status? I am not sure about that.