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 2020/08/13 17:42:45 UTC
[GitHub] [couchdb] eiri opened a new pull request #3077: Validate shard specific query params on db create request
eiri opened a new pull request #3077:
URL: https://github.com/apache/couchdb/pull/3077
## Overview
Db creation request accepts shard specific parameters `q`, `n` and `placement`, but passes them to `fabric` without validation. This leads to `badarg` crash in case of invalid parameter.
```bash
$ curl -u $(ADMIN):$(PASS) -X PUT -v "http://localhost:5984/test?q="
...
> PUT /test?q= HTTP/1.1
>
< HTTP/1.1 500 Internal Server Error
...
< Server: CouchDB/3.1.0 (Erlang OTP/20)
<
{"error":"unknown_error","reason":"badarg","ref":1446616448}
[error] 2020-08-13T17:39:05.160578Z couchdb@localhost.dev <0.18183.20> 7b4593be34 req_err(1446616448) unknown_error : badarg
[<<"erlang:list_to_integer/1">>,<<"mem3_util:q_val/1 L255">>,<<"mem3:choose_shards/3 L206">>,<<"fabric_db_create:generate_shard_map/2 L68">>,<<"fabric_db_create:go/2 L31">>,<<"chttpd_db:create_db_req/2 L398">>,<<"chttpd:handle_req_after_auth/2 L322">>,<<"chttpd:process_request/1 L305">>]
[notice] 2020-08-13T17:39:05.160852Z couchdb@localhost.dev <0.18183.20> 7b4593be34 localhost:5984 172.17.0.1 admin PUT /test?q= 500 ok 2
```
The patch adds validation on all the parameters and raises error `400` in case of failure.
## Testing recommendations
Curl request above should return error `400`.
New tests added in `chttpd_db.erl` module, isolation run
`make eunit apps=chttpd suites=chttpd_db`
## Checklist
- [x] Code is written and works correctly
- [x] Changes are covered by tests
- [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
- [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on pull request #3077: Validate shard specific query params on db create request
Posted by GitBox <gi...@apache.org>.
nickva commented on pull request #3077:
URL: https://github.com/apache/couchdb/pull/3077#issuecomment-673675749
@eiri from what I understand from reading https://docs.couchdb.org/en/stable/cluster/sharding.html#specifying-database-placement for a bit is we specifically want multiple zone placement as it would allow users to trade-off latency vs fault tolerance by placing copies in different zones so we'd need to be able to accept multiple zones in the input parameter
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on pull request #3077: Validate shard specific query params on db create request
Posted by GitBox <gi...@apache.org>.
nickva commented on pull request #3077:
URL: https://github.com/apache/couchdb/pull/3077#issuecomment-673647109
Should we allow multiple zones in the placement parameter?
```
$ http put $DB1/bad5?placement=zone1:1,zone2:2
HTTP/1.1 400 Bad Request
{
"error": "bad_request",
"reason": "The `placement` value should be in a format `zone:n`."
}
```
I don't recall ever using that option tbh, but just saw the parsing code in mem3 and saw it tokenizes the string based on "," first.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] wohali commented on pull request #3077: Validate shard specific query params on db create request
Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #3077:
URL: https://github.com/apache/couchdb/pull/3077#issuecomment-673635451
Backport to 3.x please!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] eiri commented on pull request #3077: Validate shard specific query params on db create request
Posted by GitBox <gi...@apache.org>.
eiri commented on pull request #3077:
URL: https://github.com/apache/couchdb/pull/3077#issuecomment-673784758
@wohali this is super weird, I know what you are talking about, because I have both restart buttons in BlueOcean on other Jenkins where I have an account, but missing both buttons on details page on https://ci-couchdb.apache.org. Still have `rerun` button on classical view, so maybe it's some ACL propagation delay or something.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] eiri edited a comment on pull request #3077: Validate shard specific query params on db create request
Posted by GitBox <gi...@apache.org>.
eiri edited a comment on pull request #3077:
URL: https://github.com/apache/couchdb/pull/3077#issuecomment-673636498
@wohali absolutely! How do I make Jenkins pass, it failed on an unrelated elixir test with what seems to be a race on a user creation?
**Update** Found "Rebuild" button in `classic` view.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] wohali commented on pull request #3077: Validate shard specific query params on db create request
Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #3077:
URL: https://github.com/apache/couchdb/pull/3077#issuecomment-673714153
@eiri FYI in the Blue Ocean view, you can restart the entire build (circular arrow icon at the top of the page on the right), or even just one stage of the build (just above the log view, but below the flowchart, at the right.)
I usually use the second option, it saves a few minutes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] wohali commented on pull request #3077: Validate shard specific query params on db create request
Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #3077:
URL: https://github.com/apache/couchdb/pull/3077#issuecomment-673708169
@eiri FYI you shoudl be able to log into that Jenkins instance and restart the run.
We do have a few flaky tests, still. :(
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] eiri commented on pull request #3077: Validate shard specific query params on db create request
Posted by GitBox <gi...@apache.org>.
eiri commented on pull request #3077:
URL: https://github.com/apache/couchdb/pull/3077#issuecomment-673636498
@wohali absolutely! How do I make Jenkins pass, it failed on an unrelated elixir test with what seems to be a race on a user creation?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] eiri merged pull request #3077: Validate shard specific query params on db create request
Posted by GitBox <gi...@apache.org>.
eiri merged pull request #3077:
URL: https://github.com/apache/couchdb/pull/3077
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] eiri commented on pull request #3077: Validate shard specific query params on db create request
Posted by GitBox <gi...@apache.org>.
eiri commented on pull request #3077:
URL: https://github.com/apache/couchdb/pull/3077#issuecomment-673652691
@nickva I never used it in anger myself, tbh and added here it for completion, but sure, I can update to handle multi-zones.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org