You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by eiri <gi...@git.apache.org> on 2017/01/18 14:02:43 UTC

[GitHub] couchdb-couch pull request #221: Improve errors on invalid "count" parameter...

GitHub user eiri opened a pull request:

    https://github.com/apache/couchdb-couch/pull/221

    Improve errors on invalid "count" parameter in /_uuids

    Added validation on "count" parameter to be positive and changed return code for "count" exceeding maximum from "Forbidden" to "Bad Request"
    
    COUCHDB-3273

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/cloudant/couchdb-couch validate-count-in-uuids

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/couchdb-couch/pull/221.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 #221
    
----
commit 01552406e8674d282ca3c9d58199477abb16ea78
Author: Eric Avdey <ei...@eiri.ca>
Date:   2017-01-18T13:44:15Z

    Validate that count in /_uuids is positive

commit 168caf5b79dfd6f67771dfd0f9c09f9ad8aeb760
Author: Eric Avdey <ei...@eiri.ca>
Date:   2017-01-18T13:55:19Z

    Return "Bad Request" when count in /_uuids exceeds max

----


---
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-couch issue #221: Improve errors on invalid "count" parameter in /_u...

Posted by eiri <gi...@git.apache.org>.
Github user eiri commented on the issue:

    https://github.com/apache/couchdb-couch/pull/221
  
    @kxepal I agree on tests, but it's a bit awkward, with handler been here and tests need to be in `chttpd`, so they guarantee to fail. I'll add them after this get merged and added to rebar's config, so CI will stay green.


---
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-couch pull request #221: Improve errors on invalid "count" parameter...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/221#discussion_r96643313
  
    --- Diff: src/couch_httpd_misc_handlers.erl ---
    @@ -129,7 +129,9 @@ handle_uuids_req(#httpd{method='GET'}=Req) ->
         Max = list_to_integer(config:get("uuids","max_count","1000")),
         Count = try list_to_integer(couch_httpd:qs_value(Req, "count", "1")) of
             N when N > Max ->
    -            throw({forbidden, <<"count parameter too large">>});
    +            throw({bad_request, <<"count parameter too large">>});
    +        N when N < 0 ->
    +            throw({bad_request, <<"count must be a positive integer">>});
    --- End diff --
    
    May be also edit line 138 to note that we want positive integer, not just an integer?


---
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-couch issue #221: Improve errors on invalid "count" parameter in /_u...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the issue:

    https://github.com/apache/couchdb-couch/pull/221
  
    @eiri ah, indeed, I forgot that testing is quite hard for now /: Ok, then.


---
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-couch pull request #221: Improve errors on invalid "count" parameter...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/couchdb-couch/pull/221


---
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-couch issue #221: Improve errors on invalid "count" parameter in /_u...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the issue:

    https://github.com/apache/couchdb-couch/pull/221
  
    @wohali noticed that the test associated with this, that confirms we send a 403 for the forbidden case has been broken since this change (several months!)
    
    When changing http semantics, please update tests and documentation to match.
    
    cc @eiri @kxepal.


---
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-couch issue #221: Improve errors on invalid "count" parameter in /_u...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the issue:

    https://github.com/apache/couchdb-couch/pull/221
  
    @rnewson Good reminder. Thanks! Sorry that I missed that moment /:


---
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-couch issue #221: Improve errors on invalid "count" parameter in /_u...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the issue:

    https://github.com/apache/couchdb-couch/pull/221
  
    (in the couchdb javascript test suite)


---
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-couch issue #221: Improve errors on invalid "count" parameter in /_u...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the issue:

    https://github.com/apache/couchdb-couch/pull/221
  
    I think we need some tests on this, because that's not a first time we fixing handling this parameter right.
    
    But in anyway, +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.
---