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

no reader error raised #3

Closed
benoitc opened this Issue Aug 3, 2011 · 6 comments

Comments

Projects
None yet
3 participants
@benoitc
Contributor

benoitc commented Aug 3, 2011

fabric_rpc:with_db use the couch_db:open_int function which never raise a reader error. As a consequence the reader protection of a db doesn't work.

Step to reproduce :

$ curl -XPUT 'http://admin:test@127.0.0.1:5984/testdb/_security '-d'{"admins": {
    "names": [],
    "roles": []
},
"readers": {
    "names": ["benoitc"],
    "roles": ["testdb:read"]
}}' -H"Content-Type: application/json"
$ curl -XPOST -'http://admin:test@127.0.0.1:5984/testdb' -d'{}' -H"Content-Type: application/json"

will works.

@rnewson

This comment has been minimized.

Show comment
Hide comment
@rnewson

rnewson Aug 3, 2011

Member

isnt that check done by chttpd?

Sent from my iPhone

On 3 Aug 2011, at 10:29, benoitc
reply@reply.github.com
wrote:

fabric_rpc:with_db use the couch_db:open_int function which never raise a reader error. As a consequence the reader protection of a db doesn't work.

Step to reproduce :

$ curl -XPUT 'http://admin:test@127.0.0.1:5984/testdb/_security '-d'{"admins": {
"names": [],
"roles": []
},
"readers": {
"names": ["benoitc"],
"roles": ["testdb:read"]
}}' -H"Content-Type: application/json"
$ curl -XPOST -'http://admin:test@127.0.0.1:5984/testdb' -d'{}' -H"Content-Type: application/json"

will works.

Reply to this email directly or view it on GitHub:
#3

Member

rnewson commented Aug 3, 2011

isnt that check done by chttpd?

Sent from my iPhone

On 3 Aug 2011, at 10:29, benoitc
reply@reply.github.com
wrote:

fabric_rpc:with_db use the couch_db:open_int function which never raise a reader error. As a consequence the reader protection of a db doesn't work.

Step to reproduce :

$ curl -XPUT 'http://admin:test@127.0.0.1:5984/testdb/_security '-d'{"admins": {
"names": [],
"roles": []
},
"readers": {
"names": ["benoitc"],
"roles": ["testdb:read"]
}}' -H"Content-Type: application/json"
$ curl -XPOST -'http://admin:test@127.0.0.1:5984/testdb' -d'{}' -H"Content-Type: application/json"

will works.

Reply to this email directly or view it on GitHub:
#3

@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc Aug 3, 2011

Contributor

yes it should be but chttpd doesn't open the db.

A temporary fix is :

--- a/src/chttpd_db.erl
+++ b/src/chttpd_db.erl
@@ -209,7 +209,12 @@ delete_db_req(#httpd{}=Req, DbName) ->
     end.

 do_db_req(#httpd{path_parts=[DbName|_], user_ctx=Ctx}=Req, Fun) ->
-    Fun(Req, #db{name=DbName, user_ctx=Ctx}).
+    {ok, Db} = fabric_util:get_db(DbName, [{user_ctx, Ctx}]),
+    try
+        Fun(Req, #db{name=DbName, user_ctx=Ctx})
+    after
+        catch couch_db:close(Db)
+    end.

 db_req(#httpd{method='GET',path_parts=[DbName]}=Req, _Db) ->
     % measure the time required to generate the etag, see if it's worth it
@@ -400,8 +405,9 @@ db_req(#httpd{method='PUT',path_parts=[_,<<"_security">>],user_ctx=Ctx}=Req,
     ok = fabric:set_security(Db, SecObj, [{user_ctx,Ctx}]),
     send_json(Req, {[{<<"ok">>, true}]});

-db_req(#httpd{method='GET',path_parts=[_,<<"_security">>]}=Req, Db) ->
-    send_json(Req, fabric:get_security(Db));
+db_req(#httpd{method='GET',path_parts=[_,<<"_security">>],user_ctx=Ctx}=Req,
+        Db) ->
+    send_json(Req, fabric:get_security(Db, [{user_ctx,Ctx}]));

The reason I don't pass the Db record to the db_req function for now is that all_docs hangs when doing that. I need to investigate further why.

Contributor

benoitc commented Aug 3, 2011

yes it should be but chttpd doesn't open the db.

A temporary fix is :

--- a/src/chttpd_db.erl
+++ b/src/chttpd_db.erl
@@ -209,7 +209,12 @@ delete_db_req(#httpd{}=Req, DbName) ->
     end.

 do_db_req(#httpd{path_parts=[DbName|_], user_ctx=Ctx}=Req, Fun) ->
-    Fun(Req, #db{name=DbName, user_ctx=Ctx}).
+    {ok, Db} = fabric_util:get_db(DbName, [{user_ctx, Ctx}]),
+    try
+        Fun(Req, #db{name=DbName, user_ctx=Ctx})
+    after
+        catch couch_db:close(Db)
+    end.

 db_req(#httpd{method='GET',path_parts=[DbName]}=Req, _Db) ->
     % measure the time required to generate the etag, see if it's worth it
@@ -400,8 +405,9 @@ db_req(#httpd{method='PUT',path_parts=[_,<<"_security">>],user_ctx=Ctx}=Req,
     ok = fabric:set_security(Db, SecObj, [{user_ctx,Ctx}]),
     send_json(Req, {[{<<"ok">>, true}]});

-db_req(#httpd{method='GET',path_parts=[_,<<"_security">>]}=Req, Db) ->
-    send_json(Req, fabric:get_security(Db));
+db_req(#httpd{method='GET',path_parts=[_,<<"_security">>],user_ctx=Ctx}=Req,
+        Db) ->
+    send_json(Req, fabric:get_security(Db, [{user_ctx,Ctx}]));

The reason I don't pass the Db record to the db_req function for now is that all_docs hangs when doing that. I need to investigate further why.

@kocolosk

This comment has been minimized.

Show comment
Hide comment
@kocolosk

kocolosk Aug 4, 2011

Member

Hi Benoit, I was thinking of something quite along the lines of what you're proposing here. Thanks for looking into this, it should certainly be fixed for 0.4.

Member

kocolosk commented Aug 4, 2011

Hi Benoit, I was thinking of something quite along the lines of what you're proposing here. Thanks for looking into this, it should certainly be fixed for 0.4.

@kocolosk

This comment has been minimized.

Show comment
Hide comment
@kocolosk

kocolosk Aug 12, 2011

Member

I suspect that one of the reasons you may have seen that hang in all_docs is that the Db#db.name you get back from get_db will be the name of a shard, not the name of the clustered database.

Member

kocolosk commented Aug 12, 2011

I suspect that one of the reasons you may have seen that hang in all_docs is that the Db#db.name you get back from get_db will be the name of a shard, not the name of the clustered database.

@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc Aug 12, 2011

Contributor

Yes, probably. not sure what is the proper fix for that, maybe passing the db name in the process registry ? Getting the dbname back and forth seems awkward.

Contributor

benoitc commented Aug 12, 2011

Yes, probably. not sure what is the proper fix for that, maybe passing the db name in the process registry ? Getting the dbname back and forth seems awkward.

@kocolosk

This comment has been minimized.

Show comment
Hide comment
@kocolosk

kocolosk Aug 25, 2011

Member

The patch for this issue landed earlier today in chttpd:

cloudant/chttpd#2

Thanks for the report Benoit.

Member

kocolosk commented Aug 25, 2011

The patch for this issue landed earlier today in chttpd:

cloudant/chttpd#2

Thanks for the report Benoit.

@kocolosk kocolosk closed this Aug 25, 2011

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