aws / chalice

Python Serverless Microframework for AWS
Apache License 2.0
10.6k stars 1.01k forks source link

Mapped URI parameters should be implicitly decoded / unquoted #511

Open jstell opened 7 years ago

jstell commented 7 years ago

Maybe I'm missing something, but shouldn't mapped parameter values be implicitly decoded?

CITIES_TO_STATE  = { 
    'seattle': 'WA',
    'portland': 'OR',
    'Los Angeles': 'CA' 
}
@app.route('/cities/{city}')
def state_of_city(city):
    return {'state': CITIES_TO_STATE[city]}

For example, if I'm looking for what state Los Angeles is in, I will do a GET /cities/Los%20Angeles. The above handler will not give me the expected answer, because it doesn't unquote Los%20Angeles back to Los Angeles.

Shouldn't there be a call to do city = urllib.unquote_plus(request.query_params['city']) ?

jamesls commented 7 years ago

Yes, thanks for reporting. We'll get that updated.

jstell commented 7 years ago

@jamesls Thanks for the quick response. I was envisioning this as more than a documentation issue. Shouldn't the framework implicitly do the unquote? I'm new to Chalice (and, relatively, python), but I was looking at Chalice__call__ where it creates the function arguments:

function_args = {name: event['pathParameters'][name]
                         for name in route_entry.view_args}

and thinking it could be changed to something like this:

function_args = {name: urllib.unquote_plus(event['pathParameters'][name])
                         for name in route_entry.view_args}
jamesls commented 7 years ago

The tricky part here is because we're past the 1.0 release we have to maintain backwards compatibility, given people already have to unquote these params themselves so if we start to do that for them we may break users.

I'd be interested in alternatives that could still preserve backwards compatibility.

jstell commented 7 years ago

@jamesls I agree that it's best to preserve existing behavior (at least until the next major release). Perhaps an "auto_unquote_path_parameters" configuration option, defaulted to false?

byarmis commented 4 years ago

Seconding the configuration option proposed. Also wondering if there was any discussion on other issues since this one hasn't really seen anything in the past two years.

gofabian commented 4 years ago

It is sad that this basic functionality is not included in this official AWS framework. I implemented a workaround for my project:

app = Chalice(app_name='app')

// ...

def wrap_view_function(fn):
    def unquote_param_delegate(*args, **kwargs):
        kw_copy = kwargs.copy()
        for key, value in kw_copy.items():
            kw_copy[key] = unquote_plus(value)
        return fn(*args, **kw_copy)

    return unquote_param_delegate

for path in app.routes:
    method_routes = app.routes[path]
    for method in method_routes:
        route = method_routes[method]
        route.view_function = wrap_view_function(route.view_function)