-
Notifications
You must be signed in to change notification settings - Fork 859
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
log env variables #1221
Conversation
There was a problem hiding this 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).
lib/core/logconf.c
Outdated
@@ -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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
lib/core/logconf.c
Outdated
@@ -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) |
There was a problem hiding this 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 change. Actually, you do not need to do env_var->len < 0
, since a value of size_t
is guaranteed to be non-negative.
Thank you for the changes! |
unsafe_factor
is needed