Skip to content

log env variables #1221

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

Merged
merged 9 commits into from
Mar 13, 2017
Merged

log env variables #1221

merged 9 commits into from
Mar 13, 2017

Conversation

yannick
Copy link
Contributor

@yannick 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
Copy link
Member

@kazuho kazuho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

kazuho commented Mar 13, 2017

Thank you for the changes!

@kazuho kazuho merged commit 2f51aeb into h2o:master Mar 13, 2017
kazuho added a commit that referenced this pull request Apr 11, 2017
@kazuho kazuho mentioned this pull request Apr 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants