Skip to content
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

Use dict() instead of .dict for bottle.request.query #87

Closed
wants to merge 1 commit into from

Conversation

ryandub
Copy link
Contributor

@ryandub ryandub commented Sep 12, 2015

Bottle's request.query.dict method appears to do some strange things to
query params:

Request: GET /v0/myawesomeroute?foo=12345678
bottle.request.query.dict: {'foo': ['12345678']}
dict(bottle.request.query): {'foo': '12345678'}

I'm not sure why this happens with bottle, but it has to be wrong and
makes schema validation almost impossible. Using dict() seems to fix
the problem and allows proper validation.

Bottle's request.query.dict method appears to do some strange things to
query params:

Request: GET /v0/myawesomeroute?foo=12345678
bottle.request.query.dict: {'foo': ['12345678']}
dict(bottle.request.query): {'foo': '12345678'}

I'm not sure why this happens with bottle, but it has to be wrong and
makes schema validation almost impossible. Using dict() seems to fix
the problem and allows proper validation.
@ryandub
Copy link
Contributor Author

ryandub commented Sep 12, 2015

Hmm, thinking about this some more, I see that it is probably doing this because query params can assigned value multiple times and it will create an array of these:

?foo=a&bar=b&foo=c

Would present {'foo': ['a', 'c'], 'bar': ['b']}. My code would overwrite the value with the last one, which may not be preferable.

Is it common to define multiple values for a query params this way? The RFC seems quiet on this issue: http://tools.ietf.org/html/rfc3986#section-3.4 , but Wikipedia links to some references that say allowing multiple assignments is common: https://en.wikipedia.org/wiki/Query_string#Web_forms .

Assuming that multiple assignments continue and this PR is not merged, how would one go about validating this kind of query schema? My current voluptuous schema looks as follows:

import voluptuous as v

AWESOME_SCHEMA = v.Schema({
    'foo': v.All(str, v.Length(min=12, max=20)),
})

And errors on validation:

{
    "error": {
        "code": 400,
        "description": "['foo']: expected str",
        "message": "Bad Request"
    }
}

If I change it to anticipate a list, I still get a validation error:

import voluptuous as v

AWESOME_SCHEMA = v.Schema({
    'foo': [v.All(str, v.Length(min=12, max=20))],
})
{
    "error": {
        "code": 400,
        "description": "['foo']: expected str",
        "message": "Bad Request"
    }
}

@ryandub
Copy link
Contributor Author

ryandub commented Sep 12, 2015

Looks like I just wasn't schema'ing hard enough. Everything works as expected validating on a list of params. Closing...

@ryandub ryandub closed this Sep 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant