New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

log env variables #1221

Merged
merged 9 commits into from Mar 13, 2017

Conversation

Projects
None yet
2 participants
@yannick
Contributor

yannick commented Mar 4, 2017

  • documentation
  • check if unsafe_factor is needed

@yannick yannick closed this Mar 10, 2017

@yannick yannick reopened this Mar 10, 2017

@kazuho

Thank you for the PR. I have no issue except the one comment (see below).

Show outdated Hide outdated lib/core/logconf.c
@@ -487,6 +496,13 @@ char *h2o_log_request(h2o_logconf_t *logconf, h2o_req_t *req, size_t *len, char
RESERVE(sizeof(H2O_UINT16_LONGEST_STR) - 1);
pos = append_port(pos, req->conn->callbacks->get_peername, req->conn, nullexpr);
break;
case ELEMENT_TYPE_ENV_VAR: /* %{..}e */ {
h2o_iovec_t *env_var = h2o_req_getenv(req, element->data.name.base, element->data.name.len, 0);
if (env_var == NULL || env_var->len < 1)

This comment has been minimized.

@kazuho

kazuho Mar 13, 2017

Member

Shouldn't we emit an zero-length string if len is zero? Some applications may use the existence of the environment variable as a sign of something, and in such case it is desirable to be able to distinguish between NULL and a zero-length string.

@kazuho

kazuho Mar 13, 2017

Member

Shouldn't we emit an zero-length string if len is zero? Some applications may use the existence of the environment variable as a sign of something, and in such case it is desirable to be able to distinguish between NULL and a zero-length string.

This comment has been minimized.

@yannick

yannick Mar 13, 2017

Contributor

uh, @kazuho thanks for the catch. fixed it.

@yannick

yannick Mar 13, 2017

Contributor

uh, @kazuho thanks for the catch. fixed it.

Show outdated Hide outdated lib/core/logconf.c
@@ -498,7 +498,7 @@ char *h2o_log_request(h2o_logconf_t *logconf, h2o_req_t *req, size_t *len, char
break;
case ELEMENT_TYPE_ENV_VAR: /* %{..}e */ {
h2o_iovec_t *env_var = h2o_req_getenv(req, element->data.name.base, element->data.name.len, 0);
if (env_var == NULL || env_var->len < 1)
if (env_var == NULL || env_var->len < 0)

This comment has been minimized.

@kazuho

kazuho Mar 13, 2017

Member

Thank you for the change. Actually, you do not need to do env_var->len < 0, since a value of size_t is guaranteed to be non-negative.

@kazuho

kazuho Mar 13, 2017

Member

Thank you for the change. Actually, you do not need to do env_var->len < 0, since a value of size_t is guaranteed to be non-negative.

@kazuho

This comment has been minimized.

Show comment
Hide comment
@kazuho

kazuho Mar 13, 2017

Member

Thank you for the changes!

Member

kazuho commented Mar 13, 2017

Thank you for the changes!

@kazuho kazuho merged commit 2f51aeb into h2o:master Mar 13, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

kazuho added a commit that referenced this pull request Apr 11, 2017

@kazuho kazuho referenced this pull request Apr 11, 2017

Merged

restore doc for `%{...}e` #1252

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment