You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2022/07/14 13:16:38 UTC
[GitHub] [couchdb] big-r81 opened a new pull request, #4108: Trim X-Auth-CouchDB-Roles header after reading
big-r81 opened a new pull request, #4108:
URL: https://github.com/apache/couchdb/pull/4108
Given:
```
X-Auth-CouchDB-Roles: test_1, test_2
```
This would be tokenized as
```
["test_1"," test_2"]
```
instead of
```
["test_1","test_2"]
```
Remove all whitespaces, tabs, etc. from the header after reading.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4108: Trim X-Auth-CouchDB-Roles header after reading
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4108:
URL: https://github.com/apache/couchdb/pull/4108#discussion_r921484954
##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -153,14 +153,14 @@ null_authentication_handler(Req) ->
% Headers name can be defined in local.ini. By default they are :
%
% * X-Auth-CouchDB-UserName : contain the username, (x_auth_username in
-% couch_httpd_auth section)
+% chttpd_auth section)
% * X-Auth-CouchDB-Roles : contain the user roles, list of roles separated by a
-% comma (x_auth_roles in couch_httpd_auth section)
+% comma (x_auth_roles in chttpd_auth section)
% * X-Auth-CouchDB-Token : token to authenticate the authorization (x_auth_token
-% in couch_httpd_auth section). This token is an hmac-sha1 created from secret key
+% in chttpd_auth section). This token is an hmac-sha256 created from secret key
Review Comment:
Technically I think this is part of another PR we haven't merged yet
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4108: Trim X-Auth-CouchDB-Roles header after reading
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4108:
URL: https://github.com/apache/couchdb/pull/4108#discussion_r921508306
##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -189,7 +189,7 @@ proxy_auth_user(Req) ->
Roles =
case header_value(Req, XHeaderRoles) of
undefined -> [];
- Else -> [?l2b(R) || R <- string:tokens(Else, ",")]
+ Else -> re:split(Else, "\\s*,\\s*", [trim, {return, binary}])
Review Comment:
Let's skip as discussed in `#dev` slack since we didn't have that behavior previously.
Eventually we could try to make a PR for mochiweb to update header parsing there or have our own wrapper that trims the whitespace for header values in something like couch_httpd:get_header
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4108: Trim X-Auth-CouchDB-Roles header after reading
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4108:
URL: https://github.com/apache/couchdb/pull/4108#discussion_r921398242
##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -189,7 +189,7 @@ proxy_auth_user(Req) ->
Roles =
case header_value(Req, XHeaderRoles) of
undefined -> [];
- Else -> [?l2b(R) || R <- string:tokens(Else, ",")]
+ Else -> re:split(Else, "\\s*,\\s*", [trim, {return, binary}])
Review Comment:
Aah, good catch, I misread the `trim` option, I thought it was string:trim/1...
So, Should we do something like this?
```
re:split(string:trim(Else), "\\s*,\\s*", [trim, {return, binary}])
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4108: Trim X-Auth-CouchDB-Roles header after reading
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4108:
URL: https://github.com/apache/couchdb/pull/4108#discussion_r921398242
##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -189,7 +189,7 @@ proxy_auth_user(Req) ->
Roles =
case header_value(Req, XHeaderRoles) of
undefined -> [];
- Else -> [?l2b(R) || R <- string:tokens(Else, ",")]
+ Else -> re:split(Else, "\\s*,\\s*", [trim, {return, binary}])
Review Comment:
Aah, good catch, I misread the `trim` option, I thought it was string:trim/1...
So, should we do something like this?
```
re:split(string:trim(Else), "\\s*,\\s*", [trim, {return, binary}])
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 merged pull request #4108: Trim X-Auth-CouchDB-Roles header after reading
Posted by GitBox <gi...@apache.org>.
big-r81 merged PR #4108:
URL: https://github.com/apache/couchdb/pull/4108
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4108: Trim X-Auth-CouchDB-Roles header after reading
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4108:
URL: https://github.com/apache/couchdb/pull/4108#discussion_r921264840
##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -189,7 +189,7 @@ proxy_auth_user(Req) ->
Roles =
case header_value(Req, XHeaderRoles) of
undefined -> [];
- Else -> [?l2b(R) || R <- string:tokens(Else, ",")]
+ Else -> re:split(Else, "\\s*,\\s*", [trim, {return, binary}])
Review Comment:
Did we already strip whitespace on both sides of the input (`Else`)? Otherwise we get different behavior there:
```
> re:split(" a , b,c ,d ", "\\s*,\\s*", [trim, {return, list}]).
[" a","b","c","d "]
```
vs, say
```
> string:tokens(" a , b,c, d ", " ,").
["a","b","c","d"]
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4108: Trim X-Auth-CouchDB-Roles header after reading
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4108:
URL: https://github.com/apache/couchdb/pull/4108#discussion_r921264840
##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -189,7 +189,7 @@ proxy_auth_user(Req) ->
Roles =
case header_value(Req, XHeaderRoles) of
undefined -> [];
- Else -> [?l2b(R) || R <- string:tokens(Else, ",")]
+ Else -> re:split(Else, "\\s*,\\s*", [trim, {return, binary}])
Review Comment:
Did we already strip whitespace on both sides of the input (`Else`)? Otherwise we get different behavior there:
> re:split(" a , b,c ,d ", "\\s*,\\s*", [trim, {return, list}]).
[" a","b","c","d "]
> string:tokens(" a , b,c, d ", " ,").
["a","b","c","d"]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4108: Trim X-Auth-CouchDB-Roles header after reading
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4108:
URL: https://github.com/apache/couchdb/pull/4108#discussion_r921497764
##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -189,7 +189,7 @@ proxy_auth_user(Req) ->
Roles =
case header_value(Req, XHeaderRoles) of
undefined -> [];
- Else -> [?l2b(R) || R <- string:tokens(Else, ",")]
+ Else -> re:split(Else, "\\s*,\\s*", [trim, {return, binary}])
Review Comment:
Should we use the additional string:trim here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4108: Trim X-Auth-CouchDB-Roles header after reading
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4108:
URL: https://github.com/apache/couchdb/pull/4108#discussion_r921399988
##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -189,7 +189,7 @@ proxy_auth_user(Req) ->
Roles =
case header_value(Req, XHeaderRoles) of
undefined -> [];
- Else -> [?l2b(R) || R <- string:tokens(Else, ",")]
+ Else -> re:split(Else, "\\s*,\\s*", [trim, {return, binary}])
Review Comment:
Yup, that seems to work
```
> string:trim("\n\tA \n\r\tB\n\n\t\r\n \t").
"A \n\r\tB"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4108: Trim X-Auth-CouchDB-Roles header after reading
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4108:
URL: https://github.com/apache/couchdb/pull/4108#discussion_r921484954
##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -153,14 +153,14 @@ null_authentication_handler(Req) ->
% Headers name can be defined in local.ini. By default they are :
%
% * X-Auth-CouchDB-UserName : contain the username, (x_auth_username in
-% couch_httpd_auth section)
+% chttpd_auth section)
% * X-Auth-CouchDB-Roles : contain the user roles, list of roles separated by a
-% comma (x_auth_roles in couch_httpd_auth section)
+% comma (x_auth_roles in chttpd_auth section)
% * X-Auth-CouchDB-Token : token to authenticate the authorization (x_auth_token
-% in couch_httpd_auth section). This token is an hmac-sha1 created from secret key
+% in chttpd_auth section). This token is an hmac-sha256 created from secret key
Review Comment:
Technically I think this is part of another PR yet
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org