ronantakizawa/codereview-bench huggingface dataset:
Repository: nginx/nginx
File: src/event/ngx_event_openssl.c (C)

Code before:
    return NGX_OK;
}


ngx_int_t
ngx_ssl_get_cipher_name(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s)
{
    s->data = (u_char *) SSL_get_cipher_name(c->ssl->connection);
    return NGX_OK;
}


#ifndef OPENSSL_NO_ECH
ngx_int_t
ngx_ssl_get_ech_status(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s)
{
    int    echrv = SSL_ECH_STATUS_NOT_TRIED;
    char  *inner_sni = NULL, *outer_sni = NULL, buf[PATH_MAX];

    echrv = SSL_ech_get1_status(c->ssl->connection, &inner_sni, &outer_sni);
    switch (echrv) {
    case SSL_ECH_STATUS_NOT_TRIED:
        snprintf(buf,PATH_MAX, "NOT_TRIED");
        break;
    case SSL_ECH_STATUS_FAILED:
        snprintf(buf, PATH_MAX, "TRIED_BUT_FAILED");
        break;
    case SSL_ECH_STATUS_BAD_NAME:
        snprintf(buf, PATH_MAX, "WORKED_BAD_NAME");
        break;
    case SSL_ECH_STATUS_SUCCESS:
        snprintf(buf, PATH_MAX, "SUCCESS");
        break;
    case SSL_ECH_STATUS_GREASE:
        snprintf(buf, PATH_MAX, "GREASED");
        break;
    case SSL_ECH_STATUS_BACKEND:
        snprintf(buf, PATH_MAX, "INNER");
        break;
    default:
        snprintf(buf, PATH_MAX, "ERROR");
        break;
    }
    OPENSSL_free(inner_sni);
    OPENSSL_free(outer_sni);
    s->len = ngx_strlen(buf);
    s->data = ngx_pnalloc(pool, s->len);
    ngx_memcpy(s->data, buf, s->len);
    return NGX_OK;
}


Review comment (bug):
Why not just `FAILED`?

Generally, why not just always return the status corresponding to the constant:
`SSL_ECH_STATUS_BAD_NAME` -> `BAD_NAME` etc.

Overall, the exact list of status values may be worth another look later. Currently some are missing here.

Code after:
    shm_zone = SSL_CTX_get_ex_data(ssl_ctx, ngx_ssl_session_cache_index);

    cache = shm_zone->data;
    shpool = (ngx_slab_pool_t *) shm_zone->shm.addr;

    ngx_shmtx_lock(&shpool->mutex);

    key = cache->ticket_keys;

    if (key[0].expire == 0) {

        /* initialize the current key */

        if (RAND_bytes(buf, 80) != 1) {
            ngx_ssl_error(NGX_LOG_ALERT, log, 0, "RAND_bytes() failed");
            ngx_shmtx_unlock(&shpool->mutex);
            return NGX_ERROR;
        }

        key[0].shared = 1;
        key[0].expire = expire;
        key[0].size = 80;
        ngx_memcpy(key[0].name, buf, 16);
        ngx_memcpy(key[0].hmac_key, buf + 16, 32);
        ngx_memcpy(key[0].aes_key, buf + 48, 32);

        ngx_explicit_memzero(&buf, 80);

        ngx_log_debug2(NGX_LOG_DEBUG_EVENT, log, 0,
                       "ssl ticket key: \"%*xs\"",
                       (size_t) 16, key[0].name);

        /*
         * copy the current key to the next key, as initialization of
         * the previous key will replace the current key with the next
         * key
         */

        key[2] = key[0];
    }

    if (key[1].expire < now) {

        /*
         * if the previous key is no longer needed (or not initialized),
         * replace it with the current key, replace the current key with
         * the next key, and generate new next key
         */

        key[1] = key[0];
        key[0] = key[2];
Repository: nginx/nginx
File: src/http/ngx_http_parse.c (C)

Code before:
            case '/':
                state = sw_host_start;
                break;
            default:
                return NGX_HTTP_PARSE_INVALID_REQUEST;
            }
            break;

        case sw_spaces_before_host:

            if (ch == ' ') {
                break;
            }

            /* fall through */

        case sw_host_start:

            r->host_start = p;

            if (ch == '[') {
                state = sw_host_ip_literal;
                break;
            }

            if (ch == '.' || ch == '-') {
                return NGX_HTTP_PARSE_INVALID_REQUEST;
            }

            /* fall through */

        case sw_host:
        case sw_host_dot:

            if (ch == '.') {
                if (state == sw_host_dot) {
                    return NGX_HTTP_PARSE_INVALID_REQUEST;
                }

                state = sw_host_dot;
                break;
            }

            c = (u_char) (ch | 0x20);
            if (c >= 'a' && c <= 'z') {
                state = sw_host;
                break;
            }

            if ((ch >= '0' && ch <= '9') || ch == '-') {
                state = sw_host;

Review comment (suggestion):
RFC 3986:
```
   Such a name consists of a sequence of domain labels separated by ".",
   each domain label starting and ending with an alphanumeric character
   and possibly also containing "-" characters.
```
Each label should start and end with a alphanumeric character, not just the first one.

Code after:
                break;
            default:
                return NGX_HTTP_PARSE_INVALID_REQUEST;
            }
            break;

        case sw_spaces_before_host:

            if (ch == ' ') {
                break;
            }

            /* fall through */

        case sw_host_start:

            r->host_start = p;

            if (ch == '[') {
                state = sw_host_ip_literal;
                break;
            }

            state = sw_host;

            /* fall through */

        case sw_host:

            c = (u_char) (ch | 0x20);
            if (c >= 'a' && c <= 'z') {
                break;
            }

            if ((ch >= '0' && ch <= '9') || ch == '.' || ch == '-') {
                break;
            }

            /* fall through */

        case sw_host_end:

            if (ch == ':') {
                state = sw_port;
                break;
            }

            r->host_end = p;

            if (r->method == NGX_HTTP_CONNECT) {
                return NGX_HTTP_PARSE_INVALID_REQUEST;
Repository: nginx/nginx
File: src/http/ngx_http_parse.c (C)

Code before:


ngx_table_elt_t *
ngx_http_parse_multi_header_lines_internal(ngx_http_request_t *r,
    ngx_table_elt_t *headers, ngx_str_t *name, ngx_str_t *value, u_char sep)
{
    u_char           *start, *last, *end, ch;
    ngx_table_elt_t  *h;

    for (h = headers; h; h = h->next) {

        ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
                       "parse header: \"%V: %V\"", &h->key, &h->value);

        if (name->len > h->value.len) {
            continue;
        }

        start = h->value.data;
        end = h->value.data + h->value.len;

        while (start < end) {

            if (ngx_strncasecmp(start, name->data, name->len) != 0) {
                goto skip;
            }

            for (start += name->len; start < end && *start == ' '; start++) {
                /* void */
            }

            if (value == NULL) {
                if (start == end || *start == sep) {
                    return h;
                }

                goto skip;
            }

            if (start == end || *start++ != '=') {
                /* the invalid header value */
                goto skip;
            }

            while (start < end && *start == ' ') { start++; }

            for (last = start; last < end && *last != sep; last++) {
                /* void */
            }

            value->len = last - start;

Review comment (suggestion):
- all functions (even static ones) should have prototypes
- this one is supposed to be a static function


Code after:
    }

    return NGX_ERROR;
}


ngx_table_elt_t *
ngx_http_parse_multi_header_lines(ngx_http_request_t *r,
    ngx_table_elt_t *headers, ngx_str_t *name, ngx_str_t *value)
{
    return ngx_http_parse_multi_header_lines_internal(r, headers, name, value,
                                                      ',');
}


ngx_table_elt_t *
ngx_http_parse_cookie_lines(ngx_http_request_t *r,
    ngx_table_elt_t *headers, ngx_str_t *name, ngx_str_t *value)
{
    return ngx_http_parse_multi_header_lines_internal(r, headers, name, value,
                                                      ';');
}


static ngx_table_elt_t *
ngx_http_parse_multi_header_lines_internal(ngx_http_request_t *r,
    ngx_table_elt_t *headers, ngx_str_t *name, ngx_str_t *value, u_char sep)
{
    u_char           *start, *last, *end, ch;
    ngx_table_elt_t  *h;

    for (h = headers; h; h = h->next) {

        ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
                       "parse header: \"%V: %V\"", &h->key, &h->value);

        if (name->len > h->value.len) {
            continue;
        }

        start = h->value.data;
        end = h->value.data + h->value.len;

        while (start < end) {

            if (ngx_strncasecmp(start, name->data, name->len) != 0) {
                goto skip;
            }

            for (start += name->len; start < end && *start == ' '; start++) {
                /* void */
Repository: nginx/nginx
File: src/http/modules/ngx_http_proxy_v2_module.c (C)

Code before:

        } while (!next && limit > 0);

        if (!next) {
            /*
             * if the buffer wasn't fully sent due to flow control limits,
             * preserve position for future use
             */

            if (in->buf->in_file) {
                in->buf->file_pos = file_pos;

            } else {
                in->buf->pos = pos;
            }

            break;
        }

    next:

        if (in->buf->last_buf) {
            last = 1;
        }

        if (!ngx_buf_special(in->buf) && !in->buf->in_file) {
            in->buf->last_buf = 0;
        }

        ln = in;
        in = in->next;

        ngx_free_chain(r->pool, ln);
    }

    ctx->in = in;

    if (last) {

        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
                       "http proxy output last");

        ctx->output_closed = 1;

        if (f) {
            f->flags |= NGX_HTTP_V2_END_STREAM_FLAG;

        } else {
            cl = ngx_http_proxy_v2_get_buf(r, ctx);
            if (cl == NULL) {
                return NGX_ERROR;

Review comment (suggestion):
We don't need to update last_buf if it is already unset, so the check can be moved into the previous condition.

After that, `!(true && !in_memory && !in_file) && !in_file` can be further simplified to `ngx_buf_in_memory_only(in->buf)`.

Code after:
                in->buf->pos = pos;
            }

            break;
        }

    next:

        if (in->buf->last_buf) {
            last = 1;
        }

        ln = in;
        in = in->next;

        ngx_free_chain(r->pool, ln);
    }

    ctx->in = in;

    if (last) {

        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
                       "http proxy output last");

        ctx->output_closed = 1;

        if (f) {
            f->flags |= NGX_HTTP_V2_END_STREAM_FLAG;

        } else {
            cl = ngx_http_proxy_v2_get_buf(r, ctx);
            if (cl == NULL) {
                return NGX_ERROR;
            }

            b = cl->buf;

            f = (ngx_http_proxy_v2_frame_t *) b->last;
            b->last += sizeof(ngx_http_proxy_v2_frame_t);

            f->length_0 = 0;
            f->length_1 = 0;
            f->length_2 = 0;
            f->type = NGX_HTTP_V2_DATA_FRAME;
            f->flags = NGX_HTTP_V2_END_STREAM_FLAG;
            f->stream_id_0 = (u_char) ((ctx->id >> 24) & 0xff);
            f->stream_id_1 = (u_char) ((ctx->id >> 16) & 0xff);
            f->stream_id_2 = (u_char) ((ctx->id >> 8) & 0xff);
            f->stream_id_3 = (u_char) (ctx->id & 0xff);
Repository: nginx/nginx
File: src/http/modules/ngx_http_proxy_v2_module.c (C)

Code before:
ngx_http_proxy_v2_body_output_filter(void *data, ngx_chain_t *in)
{
    ngx_http_request_t  *r = data;

    off_t                       file_pos;
    u_char                     *p, *pos, *start;
    size_t                      len, limit;
    ngx_buf_t                  *b;
    ngx_int_t                  rc;
    ngx_uint_t                 next, last;
    ngx_chain_t                *cl, *out, *ln, **ll;
    ngx_http_upstream_t        *u;
    ngx_http_proxy_v2_ctx_t    *ctx;
    ngx_http_proxy_v2_frame_t  *f;

    ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
                   "http proxy output filter");

    ctx = ngx_http_proxy_v2_get_ctx(r);

    if (ctx == NULL) {
        return NGX_ERROR;
    }

    if (!ctx->header_sent) {
        /* cleanup after previously unsent request body */

        while (ctx->in) {
            ln = ctx->in;
            ctx->in = ctx->in->next;

            ngx_free_chain(r->pool, ln);
        }

        for (ln = ctx->busy; ln; ln = ln->next) {
            ln->buf->pos = ln->buf->last;
        }
    }

    if (in) {
        if (ngx_chain_add_copy(r->pool, &ctx->in, in) != NGX_OK) {
            return NGX_ERROR;
        }
    }

    out = NULL;
    ll = &out;

    if (!ctx->header_sent) {
        /* first buffer contains headers */


Review comment (suggestion):
This can be reached only after `ngx_http_proxy_v2_reinit_request`, right? In this case it could be cheaper to clean the chain in the reinit_request itself instead of making this check on every call of output_filter.

Code after:

static ngx_int_t
ngx_http_proxy_v2_body_output_filter(void *data, ngx_chain_t *in)
{
    ngx_http_request_t  *r = data;

    off_t                       file_pos;
    u_char                     *p, *pos, *start;
    size_t                      len, limit;
    ngx_buf_t                  *b;
    ngx_int_t                  rc;
    ngx_uint_t                 next, last;
    ngx_chain_t                *cl, *out, *ln, **ll;
    ngx_http_upstream_t        *u;
    ngx_http_proxy_v2_ctx_t    *ctx;
    ngx_http_proxy_v2_frame_t  *f;

    ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
                   "http proxy output filter");

    ctx = ngx_http_proxy_v2_get_ctx(r);

    if (ctx == NULL) {
        return NGX_ERROR;
    }

    if (in) {
        if (ngx_chain_add_copy(r->pool, &ctx->in, in) != NGX_OK) {
            return NGX_ERROR;
        }
    }

    out = NULL;
    ll = &out;

    if (!ctx->header_sent) {
        /* first buffer contains headers */

        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
                       "http proxy output header");

        ctx->header_sent = 1;

        if (ctx->id != 1) {
            /*
             * keepalive connection: skip connection preface,
             * update stream identifiers
             */

            b = ctx->in->buf;
            b->pos += sizeof(ngx_http_proxy_v2_connection_start) - 1;
Repository: nginx/nginx
File: src/event/quic/ngx_event_quic_openssl_compat.c (C)

Code before:
    return 5;
}


static ngx_int_t
ngx_quic_compat_create_record(ngx_quic_compat_record_t *rec, ngx_str_t *res)
{
    ngx_str_t           ad, out;
    ngx_quic_secret_t  *secret;
    u_char              nonce[NGX_QUIC_IV_LEN];

    ad.data = res->data;
    ad.len = ngx_quic_compat_create_header(rec, ad.data, 0);

    out.len = rec->payload.len + NGX_QUIC_TAG_LEN;
    out.data = res->data + ad.len;

#ifdef NGX_QUIC_DEBUG_CRYPTO
    ngx_log_debug2(NGX_LOG_DEBUG_EVENT, rec->log, 0,
                   "quic compat ad len:%uz %xV", ad.len, &ad);
#endif

    secret = &rec->keys->secret;

    if (secret->ctx == NULL) {
        ngx_log_error(NGX_LOG_EMERG, rec->log, 0, "missing QUIC compat key");
        return NGX_ERROR;
    }

    ngx_memcpy(nonce, secret->iv.data, secret->iv.len);
    ngx_quic_compute_nonce(nonce, sizeof(nonce), rec->number);

    if (ngx_quic_crypto_seal(secret, &out, nonce, &rec->payload, &ad, rec->log)
        != NGX_OK)
    {
        return NGX_ERROR;
    }

    res->len = ad.len + out.len;

    return NGX_OK;
}


int
SSL_set_quic_transport_params(SSL *ssl, const uint8_t *params,
    size_t params_len)
{
    ngx_connection_t       *c;
    ngx_quic_compat_t      *com;
    ngx_quic_connection_t  *qc;

Review comment (bug):
NGX_LOG_EMERG is wrong here, it used to log errors during configuration parsing.

Note that SSL_provide_quic_data() errors already provide appropriate logging at the alert level, so probably it doesn't makes sense to log here anything at all, just handle missing keys and return error.

Code after:
    return 5;
}


static ngx_int_t
ngx_quic_compat_create_record(ngx_quic_compat_record_t *rec, ngx_str_t *res)
{
    ngx_str_t           ad, out;
    ngx_quic_secret_t  *secret;
    u_char              nonce[NGX_QUIC_IV_LEN];

    ad.data = res->data;
    ad.len = ngx_quic_compat_create_header(rec, ad.data, 0);

    out.len = rec->payload.len + NGX_QUIC_TAG_LEN;
    out.data = res->data + ad.len;

#ifdef NGX_QUIC_DEBUG_CRYPTO
    ngx_log_debug2(NGX_LOG_DEBUG_EVENT, rec->log, 0,
                   "quic compat ad len:%uz %xV", ad.len, &ad);
#endif

    secret = &rec->keys->secret;

    if (secret->ctx == NULL) {
        return NGX_ERROR;
    }

    ngx_memcpy(nonce, secret->iv.data, secret->iv.len);
    ngx_quic_compute_nonce(nonce, sizeof(nonce), rec->number);

    if (ngx_quic_crypto_seal(secret, &out, nonce, &rec->payload, &ad, rec->log)
        != NGX_OK)
    {
        return NGX_ERROR;
    }

    res->len = ad.len + out.len;

    return NGX_OK;
}


int
SSL_set_quic_transport_params(SSL *ssl, const uint8_t *params,
    size_t params_len)
{
    ngx_connection_t       *c;
    ngx_quic_compat_t      *com;
    ngx_quic_connection_t  *qc;
Repository: nocodb/nocodb
File: packages/nocodb/src/services/plugins.service.ts (TypeScript)

Code before:

    this.appHooksService.emit(AppEvents.PLUGIN_TEST, {
      testBody: param.body,
      req: param.req,
    });
    return await NcPluginMgrv2.test(param.body);
  }

  async pluginRead(param: { pluginId: string }) {
    return await Plugin.get(param.pluginId);
  }
  async pluginUpdate(param: {
    pluginId: string;
    plugin: PluginType;
    req: NcRequest;
  }) {
    validatePayload('swagger.json#/components/schemas/PluginReq', param.plugin);

    const pluginInfo = await Plugin.get(param.pluginId);
    if (pluginInfo?.title && pluginInfo.category) {
      await NcPluginMgrv2.test({
        title: pluginInfo.title,
        category: pluginInfo.category,
        input: param.plugin.input,
      });
    }

    const plugin = await Plugin.update(param.pluginId, param.plugin);

    this.appHooksService.emit(
      plugin.active ? AppEvents.PLUGIN_INSTALL : AppEvents.PLUGIN_UNINSTALL,
      {
        plugin,
        req: param.req,
      },
    );

    return plugin;
  }
  async isPluginActive(param: { pluginId: string }) {
    return await Plugin.isPluginActive(param.pluginId);
  }

  async webhookPluginList() {
    const plugins = await Plugin.list();

    return plugins.filter((p) =>
      ['Slack', 'Microsoft Teams', 'Discord', 'Mattermost'].includes(p.title),
    );
  }
}

Review comment (security):
Please disable the test conditionally in the case of Playwright, as we use dummy credentials in test env. Use a helper like below

```ts
export const isPlayWrightNode = () => process.env.PLAYWRIGHT_TEST === 'true';
```

Code after:
    );

    this.appHooksService.emit(AppEvents.PLUGIN_TEST, {
      testBody: param.body,
      req: param.req,
    });
    return await NcPluginMgrv2.test(param.body);
  }

  async pluginRead(param: { pluginId: string }) {
    return await Plugin.get(param.pluginId);
  }
  async pluginUpdate(param: {
    pluginId: string;
    plugin: PluginType;
    req: NcRequest;
  }) {
    validatePayload('swagger.json#/components/schemas/PluginReq', param.plugin);

    const pluginInfo = await Plugin.get(param.pluginId);
    if (pluginInfo?.title && pluginInfo.category && !isPlayWrightNode()) {
      await NcPluginMgrv2.test({
        title: pluginInfo.title,
        category: pluginInfo.category,
        input: param.plugin.input,
      });
    }

    const plugin = await Plugin.update(param.pluginId, param.plugin);

    this.appHooksService.emit(
      plugin.active ? AppEvents.PLUGIN_INSTALL : AppEvents.PLUGIN_UNINSTALL,
      {
        plugin,
        req: param.req,
      },
    );

    return plugin;
  }
  async isPluginActive(param: { pluginId: string }) {
    return await Plugin.isPluginActive(param.pluginId);
  }

  async webhookPluginList() {
    const plugins = await Plugin.list();

    return plugins.filter((p) =>
      ['Slack', 'Microsoft Teams', 'Discord', 'Mattermost'].includes(p.title),
    );
  }
Repository: nocodb/nocodb
File: packages/nocodb/src/services/plugins.service.ts (TypeScript)

Code before:

  async pluginTest(param: { body: PluginTestReqType; req: NcRequest }) {
    validatePayload(
      'swagger.json#/components/schemas/PluginTestReq',
      param.body,
    );

    this.appHooksService.emit(AppEvents.PLUGIN_TEST, {
      testBody: param.body,
      req: param.req,
    });
    return await NcPluginMgrv2.test(param.body);
  }

  async pluginRead(param: { pluginId: string }) {
    return await Plugin.get(param.pluginId);
  }
  async pluginUpdate(param: {
    pluginId: string;
    plugin: PluginType;
    req: NcRequest;
  }) {
    validatePayload('swagger.json#/components/schemas/PluginReq', param.plugin);

    const pluginInfo = await Plugin.get(param.pluginId);
    if (pluginInfo?.title && pluginInfo.category) {
      await NcPluginMgrv2.test({
        title: pluginInfo.title,
        category: pluginInfo.category,
        input: param.plugin.input,
      });
    }

    const plugin = await Plugin.update(param.pluginId, param.plugin);

    this.appHooksService.emit(
      plugin.active ? AppEvents.PLUGIN_INSTALL : AppEvents.PLUGIN_UNINSTALL,
      {
        plugin,
        req: param.req,
      },
    );

    return plugin;
  }
  async isPluginActive(param: { pluginId: string }) {
    return await Plugin.isPluginActive(param.pluginId);
  }

  async webhookPluginList() {
    const plugins = await Plugin.list();

Review comment (suggestion):
```suggestion
    if (pluginInfo?.title && pluginInfo.category  && !isPlayWrightNode()) {
```

Code after:
  }

  async pluginTest(param: { body: PluginTestReqType; req: NcRequest }) {
    validatePayload(
      'swagger.json#/components/schemas/PluginTestReq',
      param.body,
    );

    this.appHooksService.emit(AppEvents.PLUGIN_TEST, {
      testBody: param.body,
      req: param.req,
    });
    return await NcPluginMgrv2.test(param.body);
  }

  async pluginRead(param: { pluginId: string }) {
    return await Plugin.get(param.pluginId);
  }
  async pluginUpdate(param: {
    pluginId: string;
    plugin: PluginType;
    req: NcRequest;
  }) {
    validatePayload('swagger.json#/components/schemas/PluginReq', param.plugin);

    const pluginInfo = await Plugin.get(param.pluginId);
    if (pluginInfo?.title && pluginInfo.category && !isPlayWrightNode()) {
      await NcPluginMgrv2.test({
        title: pluginInfo.title,
        category: pluginInfo.category,
        input: param.plugin.input,
      });
    }

    const plugin = await Plugin.update(param.pluginId, param.plugin);

    this.appHooksService.emit(
      plugin.active ? AppEvents.PLUGIN_INSTALL : AppEvents.PLUGIN_UNINSTALL,
      {
        plugin,
        req: param.req,
      },
    );

    return plugin;
  }
  async isPluginActive(param: { pluginId: string }) {
    return await Plugin.isPluginActive(param.pluginId);
  }

  async webhookPluginList() {