Request.current_route_url drops _query

[rbu]

Request.current_route_url is an easy way to generate the current route, but add/update a few parameters. This works fine for elements in the matchdict, but not at all for _query parameters. I know they're very 1990's, but they are still useful for optional filters, etc.

Expected behavior would be:
URI = "http://example.com/fnord?id=23"

> request.current_route_url(_query=dict(page=2))
"http://example.com/fnord?id=23&page=2"
> request.current_route_url()
"http://example.com/fnord?id=23"

Current behavior:

> request.current_route_url(_query=dict(page=2))
"http://example.com/fnord?page=2"
> request.current_route_url()
"http://example.com/fnord"

[mmerickel]

I agree that current_route_url should preserve the original query string. However if you specify your own _query argument then it will override whatever was there before.

qs = request.GET.copy()
qs.update({'page': 2})
request.current_route_url(_query=qs.items())
'http://example.com/fnord?id=23&page=2'

[rbu]

It would very convenient for current_route_url were to encapsulate the merging. After all, that is the behavior of the matchdict arguments, and it seems to me this covers how the function is currently used. If needed, the replace behavior could be exposed with another argument.

[merwok]

Likewise, a custom port number will be dropped.

[digitalresistor]

@merwork What do you mean by a custom port number? I haven't found that request.current_route_url drops the port number.

[merwok]

I mean non-default: if you run with waitress on port 6543 for example, route_url will generate http://localhost/etc

(I’ll need to check with latest pyramid, and also current_route_url vs. route_url)

[digitalresistor]

Neither request.current_route_url or request.route_url will drop the port number. I use both extensively throughout my code base, and I run on localhost for testing, on the default port of 6543.

[mmerickel]

The port number issue sounds like a case of not properly forwarding information to the wsgi environ from your reverse proxy. This is a classic issue with https, port numbers and path prefixes behind reverse proxies.

[odontomachus]

Currently, request.current_route_url strips the query string. Passing it the _query parameter overrides the default behavior which is inconsistent.

An option would be to have an extra parameter _with_current_qs which would allow for:
1- _with_current_qs=False, no _query parameter: current behavior (default)
2- _with_current_qs=False, _query={a=b}: current behavior (just ?page=4)
3- _with_current_qs=True, no _query: url with current query string (?id=23)
4- _with_current_qs=True, _query:={a=b}: url with current query string and merged _query (?id=23&page=4)

This would not break backward compatibility and would provide the options. However, it is not very intuitive and should be properly documented. Or we could make _with_current_qs=True by default and break backward compatibility but provide a more intuitive behavior.

[blaflamme]

Or the behaviour could be changed to return by default the query string and pass _query=None if you want nothing (current behaviour), and pass {a=b} if you want to override the values in the query string.

[mmerickel]

The merging operation is unintuitive for the query string because you can specify each parameter more than once which is valid in HTTP and the reason the _query argument accepts an iterable of 2-tuples as well as just a dict. For example:

items = [('q', '1'), ('q', '2')]
url = request.route_url('foo', _query=items) # -> http://example.com/foo?q=1&q=2

To me the things to do in this situation are:

• Override the pre-existing query string.
• Append the new params to the old params.
• Overwrite duplicate old params with the new params, and append any non-duplicate params.

The override option could be done by telling the user to just use route_url instead of current_route_url. The ambiguity in the last two options could be something like _allow_query_duplicates=True.

To me the sane option is still to just override the whole query string if specified.

[jeffszusz]

So should we provide a _with_query = True/False flag for people that -want- the query string included?

[blaflamme]

@mmerickel You're right about the sane option of overriding the whole query string. How's about request.current_url returning the query string? Should it be the default behaviour or like @jeffszusz proposes to pass a new param? To me it seems expected that the returned url should contains the query string, like request.url, but it breaks up bw compatibility.

[mmerickel]

I think it's a bug that the current current_route_url does not include the query string from the current url so as far as backward compatibility I personally think it should be fixed. @mcdonc may disagree with me though.

[blaflamme]

I'm +1 on this, lets see what @mcdonc thinks :)

[merwok]

+1 :)

[mcdonc]

I agree it's a bug and should be fixed.

[digitalresistor]

👍

[goodwillcoding]

+1

[mmerickel]

Fixed via #1081.

In the future if a real merge operation is desired this can probably be done via some boolean flag to current_route_url().