Friday, 18 April 2008

Nginx и потенциальные проблемы, мимо которых пока пролетают все, файлы: ответ

В рассылке nginx’а подняли тему о моем наезде на него.

Ответ автора был более менее понятен, но дальше меня обвинили в невнимательности. Возможно я и не внимательный, не отрицаю, но хочу поделиться тем, как работает упомянутый там ngx_http_map_uri_to_path.

Для начала нам потребуется код этой функции:

u_char *
ngx_http_map_uri_to_path(ngx_http_request_t *r, ngx_str_t *path,
    size_t *root_length, size_t reserved)
{
    u_char                    *last;
    size_t                     alias;
    ngx_http_core_loc_conf_t  *clcf;

    clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);

    alias = clcf->alias ? clcf->name.len : 0;

    if (alias && !r->valid_location) {
        ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
                      "\"alias\" could not be used in location \"%V\" "
                      "where URI was rewritten", &clcf->name);
        return NULL;
    }

    reserved += r->uri.len — alias + 1;

    if (clcf->root_lengths == NULL) {

        *root_length = clcf->root.len;

        path->len = clcf->root.len + reserved;

        path->data = ngx_palloc(r->pool, path->len);
        if (path->data == NULL) {
            return NULL;
        }

        last = ngx_copy(path->data, clcf->root.data, clcf->root.len);

    } else {
        if (ngx_http_script_run(r, path, clcf->root_lengths->elts, reserved,
                                clcf->root_values->elts)
            == NULL)
        {
            return NULL;
        }

        if (ngx_conf_full_name((ngx_cycle_t *) ngx_cycle, path, 0)== NGX_ERROR)
        {
            return NULL;
        }

        *root_length = path->len — reserved;
        last = path->data + *root_length;
    }

    last = ngx_cpystrn(last, r->uri.data + alias, r->uri.len — alias + 1);

    return last;
}

Ничего криминально или странного тут нет. Интересен тут вызов ngx_cpystrn. Для показания интереса показываю и ее код:

u_char *
ngx_cpystrn(u_char *dst, u_char *src, size_t n)
{
    if (n == 0) {
        return dst;
    }

    for ( /* void */ ; --n; dst++, src++) {
        *dst = *src;

        if (*dst == '\0') {
            return dst;
        }
    }

    *dst = '\0';

    return dst;
}

На первый взгляд ничего криминального тут нет. Копируем байты откуда‑то куда‑то в количестве n. Но в конце стоит

*dst = '\0';

Вот это не совсем понятно. Если бы функция как-то предупреждала что ей нужен dst размером n+1 минимум, то претензий бы не было, но в таком виде, это просто потенциальные долгие проблемы при отладке. Разве нет? Либо сегфол, но в случае его, об этом узнается много быстрее. К функции осталось одна притензия: название не очевидное, слишком сильно коррелирует с strcpyn. Т.е. надо предупреждать что она копирует n-1 символо и ставит терминирующий '\0'.

Write on: 12:09 | 4 comments | | tags: , , , | permalink |
Add post to:   Delicious Reddit Slashdot Digg Technorati Google


Add comment

Comments

Sergey Shepelev 29.04.2008 19:57

Второе слово в статье должно быть ”рассылке”.

reply
Kirill A. Korinskiy 30.04.2008 11:13

Спасибо, исправил.

reply
dkrotx 30.04.2008 14:03

Копируем байты откуда‑то куда‑то в количестве n. Но в конце стоит” … “что ей нужен dst размером n+1 минимум” будте внимательней — сколько раз по-вашему выполнится цикл? Реализация автора верна — это гарантированное завершение строки 0.

reply

Comment form for «Nginx и потенциальные проблемы, мимо которых пока пролетают все, файлы: ответ»

Required. 30 chars of fewer.

Required.

Kirill A. Korinskiy 30.04.2008 21:58

Гм. N. Спасибо. Я почему-то прочитал это как n--.

Но притензию к не очевидному названию это не снимает.

reply