You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by iilyak <gi...@git.apache.org> on 2016/03/21 21:11:14 UTC
[GitHub] couchdb-chttpd pull request: 2973 log user
GitHub user iilyak opened a pull request:
https://github.com/apache/couchdb-chttpd/pull/109
2973 log user
Change the log format from `chttpd` to include username.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloudant/couchdb-chttpd 2973-log-user
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-chttpd/pull/109.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #109
----
commit e71867d94b8252c1bd9d986030acabf22a4bbc62
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Date: 2016-03-21T19:23:31Z
Log user name of request initiator
Change the log format from chttpd to include username.
The new format is (space separated):
Nonce, Peer, Host, UserName, Method, RawUri, Code, Status, RequestTime
COUCHDB-2973
commit 0b71d1e0f9c78bb42dd2c2148de6fcdddd33b8c5
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Date: 2016-03-21T20:06:28Z
Add `log_format_test` test case
COUCHDB-2973
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-210500204
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-199843431
we could apply url encoding before logging it. That way administrators can decode to the real value but we don't have to worry about any unicode trickery.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-210502309
+1, but need to rebase. Bonus +1 if empty space will get reduced (;
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#discussion_r56889530
--- Diff: src/chttpd.erl ---
@@ -1074,3 +1075,53 @@ respond_(#httpd{mochi_req = MochiReq}, Code, Headers, _Args, start_response) ->
MochiReq:start_response({Code, Headers});
respond_(#httpd{mochi_req = MochiReq}, Code, Headers, Args, Type) ->
MochiReq:Type({Code, Headers, Args}).
+
+get_user(#httpd{user_ctx = #user_ctx{name = User}}) -> User;
+get_user(#httpd{user_ctx = undefined}) -> "undefined".
--- End diff --
What's the case for undefined? I thought we always have user_ctx set...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#discussion_r57214872
--- Diff: src/chttpd.erl ---
@@ -1074,3 +1075,60 @@ respond_(#httpd{mochi_req = MochiReq}, Code, Headers, _Args, start_response) ->
MochiReq:start_response({Code, Headers});
respond_(#httpd{mochi_req = MochiReq}, Code, Headers, Args, Type) ->
MochiReq:Type({Code, Headers, Args}).
+
+get_user(#httpd{user_ctx = #user_ctx{name = User}}) ->
+ mochiweb_util:quote_plus(User);
--- End diff --
couch_util:url_encode ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-199785630
@kxepal: I did create a jira ticket [COUCHDB-2974](https://issues.apache.org/jira/browse/COUCHDB-2974) to track utf-8 support.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-199785895
> @kxepal: Oh yes, user names "null" and "undefined" will cause a lot of fun.
Would you recommend changing `"undefined"` -> `"null"`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#discussion_r59890962
--- Diff: src/chttpd.erl ---
@@ -1074,3 +1075,60 @@ respond_(#httpd{mochi_req = MochiReq}, Code, Headers, _Args, start_response) ->
MochiReq:start_response({Code, Headers});
respond_(#httpd{mochi_req = MochiReq}, Code, Headers, Args, Type) ->
MochiReq:Type({Code, Headers, Args}).
+
+get_user(#httpd{user_ctx = #user_ctx{name = User}}) ->
+ couch_util:url_encode(User);
+get_user(#httpd{user_ctx = undefined}) ->
+ "undefined".
+
+-ifdef(TEST).
+-include_lib("eunit/include/eunit.hrl").
+
+log_format_test() ->
+ ?assertEqual(
+ "nonce 127.0.0.1 127.0.0.1:15984 undefined "
+ "GET /_cluster_setup 201 ok 10000",
+ test_log_request("/_cluster_setup", undefined)),
+ ?assertEqual(
+ "nonce 127.0.0.1 127.0.0.1:15984 user_foo "
+ "GET /_all_dbs 201 ok 10000",
+ test_log_request("/_all_dbs", #user_ctx{name = <<"user_foo">>})),
+
+ %% Utf8Name = unicode:characters_to_binary(Something),
+ Utf8User = <<227,130,136,227,129,134,227,129,147,227,129,157>>,
+ ?assertEqual(
+ "nonce 127.0.0.1 127.0.0.1:15984 %E3%82%88%E3%81%86%E3%81%93%E3%81%9D "
+ "GET /_all_dbs 201 ok 10000",
+ test_log_request("/_all_dbs", #user_ctx{name = Utf8User})),
+
+ ok.
+
+test_log_request(RawPath, UserCtx) ->
+ Headers = mochiweb_headers:make([{"HOST", "127.0.0.1:15984"}]),
+ MochiReq = mochiweb_request:new(socket, [], 'POST', RawPath, version, Headers),
+ Req = #httpd{
+ mochi_req = MochiReq,
+ begin_ts = {1458,588713,124003},
+ original_method = 'GET',
+ peer = "127.0.0.1",
+ nonce = "nonce",
+ user_ctx = UserCtx
+ },
+ Resp = #httpd_resp{
+ end_ts = {1458,588723,124303},
+ code = 201,
+ status = ok
+ },
+ ok = meck:new(couch_log, [passthrough]),
+ ok = meck:expect(couch_log, notice, fun(Format, Args) ->
+ lists:flatten(io_lib:format(Format, Args))
+ end),
+ Message = maybe_log(Req, Resp),
+ ok = meck:unload(couch_log),
+ Message.
+
+
+
--- End diff --
Reserved for future tests? (:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-199795675
>>Oh yes, user names "null" and "undefined" will cause a lot of fun.
> Would you recommend changing "undefined" -> "null"?
No, I mean that user with name "null" will looks in logs as the anonymous user - so you'll never trace him. Same story with "undefined". I don't think there is any solution except to quote the name or do something to distinguish authed user from anonymous ones.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-199796433
As for UTF-8: yes, we do. More over, we already support UTF-8 in basic auth header regardless the fact that RFC assumes ASCII there.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-199779143
@kxepal Supporting utf-8 is a very valid concern. Do we actually support it? It doesn't seem possible to transmit utf-8 in a http header. We use basic auth which is based on headers. There is a new [RFC7617](https://datatracker.ietf.org/doc/rfc7617/) which is going to support utf-8. But currently it is not supported. Therefore I do believe that we shouldn't have any utf-8 users in the wild. As utf-8 support is slowly coming maybe we should consider userid validation on couch side to sanitize user's input before it became a problem. The proposed [RFC7613](https://datatracker.ietf.org/doc/rfc7613/?include_text=1) defines what can be in a userid and what shouldn't be there.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-199840399
@iilyak I would prefer json or url encoded way. Both are fine to notice tricky bits, but each is useful for further copy-paste into some script to work with it and both provides more-or-less readable close to real variants of username. Plain ASCII ones will remain plain ASCII ones.
However, that could be not very friendly for the end users (admins/devops) to work with such kind of data. Need to think about and call for more opinions around. For instance, what tools ElasticSearch could provide to work with such kind of encoded usernames?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-199473028
Oh yes, user names `"null"` and `"undefined"` will cause a lot of fun.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-199914422
+1 url encoding then. That's better than json as you can simply copy-paste the name to url and get the doc, it's still readable for ASCII names and quite easy to convert into UTF-8 string if need.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#discussion_r56890826
--- Diff: src/chttpd.erl ---
@@ -1074,3 +1075,53 @@ respond_(#httpd{mochi_req = MochiReq}, Code, Headers, _Args, start_response) ->
MochiReq:start_response({Code, Headers});
respond_(#httpd{mochi_req = MochiReq}, Code, Headers, Args, Type) ->
MochiReq:Type({Code, Headers, Args}).
+
+get_user(#httpd{user_ctx = #user_ctx{name = User}}) -> User;
+get_user(#httpd{user_ctx = undefined}) -> "undefined".
--- End diff --
Nvm, I see it. Thought it's strange that this value is unstable depending on what operation you proceed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-199466642
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-199469748
May be one addition to not log user names as UTF-8, but in some JSON to not get fooled by unicode tricky usernames or get log lines broken by newlines in username.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/109#issuecomment-199838409
@kxepal: Should we log `hex(base64(name))` instead then? Even ASCII could have `\a` or `\r` which could brake the log lines. It such a pity that we historically do not validate user names.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 2973 log user
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-chttpd/pull/109
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---