Should we allow requests with non-url encoded strings?

[vsudilov]

Noticed this in one of the logs, which is possible to reproduce if one passes in a ":" to directly to the api.

ValueError: Error trying to decode a non urlencoded string. Found invalid characters: set([u':']) in the string: 'q=bibcode:2015arXiv150301104A&fl=title'. Please ensure the request/response body is x-www-form-urlencoded

References adsabs/bumblebee#123

[romanchyla]

Hmm, good question. Bumblebee is definitely wrong, but do you think it is the oath being too strict? On Apr 27, 2015 11:09 AM, "Vladimir Sudilovsky" notifications@github.com wrote:

Noticed this in one of the logs, which is possible to reproduce if one passes in a ":" to directly to the api.

ValueError: Error trying to decode a non urlencoded string. Found invalid characters: set([u':']) in the string: 'q=bibcode:2015arXiv150301104A&fl=title'. Please ensure the request/response body is x-www-form-urlencoded

References adsabs/bumblebee#123 https://github.com/adsabs/bumblebee/issues/123

— Reply to this email directly or view it on GitHub https://github.com/adsabs/adsws/issues/51.

[vsudilov]

I'd like to know the reasons behind disallowing those characters, but they definitely need to be allowed if don't want to put up an artificial barrier for our API users. It's either that or re-write/re-implement the solr search syntax, which is even more problematic.

[romanchyla]

This has nothing to do with solr syntax, it is about set of characters that the api accepts unencoded. Bbb should properly encode them. But we need to adhere to decide/know which sets we support (I assume we follow standard set) On Apr 27, 2015 4:13 PM, "Vladimir Sudilovsky" notifications@github.com wrote:

I'd like to know the reasons behind disallowing those characters, but they definitely need to be allowed if don't want to put up an artificial barrier for our API users. It's either that or re-write/re-implement the solr search syntax, which is even more problematic.

— Reply to this email directly or view it on GitHub https://github.com/adsabs/adsws/issues/51#issuecomment-96846663.

[vsudilov]

Added ":" to the common.py.monkeypatch for the moment

[spacemansteve]

This still happens. We return a http status code of 500/internal server error. I really wish we returned a 400 for bad input. @romanchyla Do you agree?
There has been discussion of this being handled in werkzeug but I don't know it has happened.
Here's the stack trace of an error:

  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1982, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1614, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/usr/local/lib/python2.7/dist-packages/flask_restful/__init__.py", line 273, in error_router
    return original_handler(e)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1517, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1612, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1598, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/usr/local/lib/python2.7/dist-packages/flask_oauthlib/provider/oauth2.py", line 529, in decorated
    uri, http_method, body, headers, scopes
  File "/usr/local/lib/python2.7/dist-packages/oauthlib/oauth2/rfc6749/endpoints/base.py", line 64, in wrapper
    return f(endpoint, uri, *args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/oauthlib/oauth2/rfc6749/endpoints/resource.py", line 68, in verify_request
    request = Request(uri, http_method, body, headers)
  File "/usr/local/lib/python2.7/dist-packages/oauthlib/common.py", line 395, in __init__
    self._params.update(dict(urldecode(self.uri_query)))
  File "/usr/local/lib/python2.7/dist-packages/oauthlib/common.py", line 129, in urldecode
    raise ValueError(error % (set(query) - urlencoded, query))
ValueError: Error trying to decode a non urlencoded string. Found invalid characters: set([u'[', u']']) in the string: 'fl=Dlinks_data%2C[citations]%2Ckeyword%2Cproperty%2Cfirst_author%2Cyear%2Cissn%2Cisbn%2Ctitle%2Caff%2Cabstract%2Cbibcode%2Cpub%2Cvolume%2Cauthor%2Cissue%2Cpubdate%2Cdoi%2Cpage%2Cesources%2Cdata=q=identifier:2017ApJ...836L..14M&rows=1'. Please ensure the request/response body is x-www-form-urlencoded.```

[romanchyla]

I agree that 400 is more appropriate and I think it can be had more easily; by adding an appropriate error handler that can process ValueError exceptions.

I'd not like to add [] to the list of characters that don't need to be encoded (precisely because I don't know if it has some side effects; and it is confusing in the end if some standard chars are and some are not required to be url encoded)

On Fri, Nov 22, 2019 at 10:55 AM Steve McDonald notifications@github.com wrote:

This still happens. We return a http status code of 500/internal server error. I really wish we returned a 400 for bad input. @romanchyla https://github.com/romanchyla Do you agree? There has been (discussion)[https://github.com/pallets/werkzeug/pull/1363 https://github.com/pallets/werkzeug/pull/1363] of this being handled in werkzeug but I don't know it has happened. Here's the stack trace of an error:

File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1982, in wsgi_app response = self.full_dispatch_request() File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1614, in full_dispatch_request rv = self.handle_user_exception(e) File "/usr/local/lib/python2.7/dist-packages/flask_restful/init.py", line 273, in error_router return original_handler(e) File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1517, in handle_user_exception reraise(exc_type, exc_value, tb) File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1612, in full_dispatch_request rv = self.dispatch_request() File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1598, in dispatch_request return self.view_functionsrule.endpoint File "/usr/local/lib/python2.7/dist-packages/flask_oauthlib/provider/oauth2.py", line 529, in decorated uri, http_method, body, headers, scopes File "/usr/local/lib/python2.7/dist-packages/oauthlib/oauth2/rfc6749/endpoints/base.py", line 64, in wrapper return f(endpoint, uri, *args, **kwargs) File "/usr/local/lib/python2.7/dist-packages/oauthlib/oauth2/rfc6749/endpoints/resource.py", line 68, in verify_request request = Request(uri, http_method, body, headers) File "/usr/local/lib/python2.7/dist-packages/oauthlib/common.py", line 395, in init self._params.update(dict(urldecode(self.uri_query))) File "/usr/local/lib/python2.7/dist-packages/oauthlib/common.py", line 129, in urldecode raise ValueError(error % (set(query) - urlencoded, query)) ValueError: Error trying to decode a non urlencoded string. Found invalid characters: set([u'[', u']']) in the string: 'fl=Dlinks_data%2C[citations]%2Ckeyword%2Cproperty%2Cfirst_author%2Cyear%2Cissn%2Cisbn%2Ctitle%2Caff%2Cabstract%2Cbibcode%2Cpub%2Cvolume%2Cauthor%2Cissue%2Cpubdate%2Cdoi%2Cpage%2Cesources%2Cdata=q=identifier:2017ApJ...836L..14M&rows=1'. Please ensure the request/response body is x-www-form-urlencoded.```

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/adsabs/adsws/issues/51?email_source=notifications&email_token=AADERETYMP646S3IQU7HUQLQU76F3A5CNFSM4BBHSV6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE6BVNQ#issuecomment-557587126, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADEREUU433HH75LQJ3YNYLQU76F3ANCNFSM4BBHSV6A .