You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by mayya-sharipova <gi...@git.apache.org> on 2015/08/15 01:25:13 UTC
[GitHub] couchdb-chttpd pull request: check POST requests for valid json he...
GitHub user mayya-sharipova opened a pull request:
https://github.com/apache/couchdb-chttpd/pull/56
check POST requests for valid json header
validate that all POST requests with json body must have also have valid
json header: {"Content-Type": "application/json"}
This ensures a basic protection against CSRF
JIRA: COUCHDB-2775
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloudant/couchdb-chttpd 2775-post-valid-json-header
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-chttpd/pull/56.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 #56
----
commit fbdfd9085649092b9ff39e936bf233c258335a52
Author: Mayya Sharipova <ma...@ca.ibm.com>
Date: 2015-08-14T23:18:32Z
check POST requests for valid json header
validate that all POST requests with json body must have also have valid
json header: {"Content-Type": "application/json"}
This ensures a basic protection against CSRF
JIRA: COUCHDB-2775
----
---
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: check POST requests for valid json he...
Posted by mayya-sharipova <gi...@git.apache.org>.
Github user mayya-sharipova commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/56#discussion_r37138890
--- Diff: src/chttpd_db.erl ---
@@ -362,7 +362,6 @@ db_req(#httpd{path_parts=[_,<<"_ensure_full_commit">>]}=Req, _Db) ->
db_req(#httpd{method='POST',path_parts=[_,<<"_bulk_docs">>], user_ctx=Ctx}=Req, Db) ->
couch_stats:increment_counter([couchdb, httpd, bulk_requests]),
- couch_httpd:validate_ctype(Req, "application/json"),
--- End diff --
Because after this line follows: {JsonProps} = chttpd:json_body_obj(Req), and the function json_body_obj already having a check for header.
---
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: check POST requests for valid json he...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-chttpd/pull/56
---
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: check POST requests for valid json he...
Posted by mayya-sharipova <gi...@git.apache.org>.
Github user mayya-sharipova commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/56#issuecomment-131887523
Thanks for the comments. Not sure what LGFM means. For the other apps: I plan to validate headers for POST requests also in couch, couch_mrview and mango.
---
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: check POST requests for valid json he...
Posted by mayya-sharipova <gi...@git.apache.org>.
Github user mayya-sharipova commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/56#discussion_r37191398
--- Diff: src/chttpd_db.erl ---
@@ -362,7 +362,6 @@ db_req(#httpd{path_parts=[_,<<"_ensure_full_commit">>]}=Req, _Db) ->
db_req(#httpd{method='POST',path_parts=[_,<<"_bulk_docs">>], user_ctx=Ctx}=Req, Db) ->
couch_stats:increment_counter([couchdb, httpd, bulk_requests]),
- couch_httpd:validate_ctype(Req, "application/json"),
--- End diff --
Thanks for the detailed comments, this makes sense. I have updated the commit to reflect them.
---
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: check POST requests for valid json he...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/56#issuecomment-131910181
@mayya-sharipova Looks Good For Me (:
@rnewson +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: check POST requests for valid json he...
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/56#discussion_r37134761
--- Diff: src/chttpd_db.erl ---
@@ -362,7 +362,6 @@ db_req(#httpd{path_parts=[_,<<"_ensure_full_commit">>]}=Req, _Db) ->
db_req(#httpd{method='POST',path_parts=[_,<<"_bulk_docs">>], user_ctx=Ctx}=Req, Db) ->
couch_stats:increment_counter([couchdb, httpd, bulk_requests]),
- couch_httpd:validate_ctype(Req, "application/json"),
--- End diff --
Why did you remove the check here?
---
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: check POST requests for valid json he...
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/56#discussion_r37139946
--- Diff: src/chttpd_db.erl ---
@@ -362,7 +362,6 @@ db_req(#httpd{path_parts=[_,<<"_ensure_full_commit">>]}=Req, _Db) ->
db_req(#httpd{method='POST',path_parts=[_,<<"_bulk_docs">>], user_ctx=Ctx}=Req, Db) ->
couch_stats:increment_counter([couchdb, httpd, bulk_requests]),
- couch_httpd:validate_ctype(Req, "application/json"),
--- End diff --
mmm...I think it's conceptually wrong: `chttpd:json_body_obj` ensures that request body is a JSON object, but there is more general `json_body` function which need to be insured that request body _is_ a JSON value.
However, both shouldn't care about mime type - that's not their work and this prevents to reuse these function if request body is a JSON, but mime type is not a `application/json`. For instance, it could be `application/vnd.cloudant+json`, but because of `chttpd:json_body` does additional wrong job, you'll have to create your own function to decode JSON.
Single responsibility is a good idea to follow.
---
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: check POST requests for valid json he...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/56#issuecomment-131939347
I'd rather this was chttpd:validate_ctype instead, but it looks good otherwise. Let's add validate_ctype to chttpd and have that function, and only that one, call down to couch_httpd.
---
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: check POST requests for valid json he...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/56#issuecomment-131836495
LGFM. Did you check other apps which provides chttpd endpoints for POST content-type validation?
---
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: check POST requests for valid json he...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/56#issuecomment-137133237
+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.
---