behave-django icon indicating copy to clipboard operation
behave-django copied to clipboard

Investigate potential bug in `environment.get_url()`

Open bittner opened this issue 3 years ago • 0 comments

As mentioned in https://github.com/behave/behave-django/pull/140#pullrequestreview-932253596, odds are that we have a problem with the implementation of environment.get_url().

  • The to argument should correspond to the first positional parameter of the Django shortcut's resolve_url function. Our implementation may break when to is something else than a string.
  • Specifically, when get_absolute_url() returns a URL containing a protocol and a hostname or IP address then there will be two base_urls in the resulting URL string.

Pylint (correctly) complains about to being a keyword-argument used as a positional argument, and to being a bad name. In Django's resolve_url implementation, to is a positional argument, but we want it to be optional (not necessarily a keyword-argument, strictly speaking), e.g.

# returns the value of `base_url`
context.get_url()
# returns base_url + '/admin'
context.get_url('/admin')

When to Is a Model or a View Name

The problem may occur when the argument passed to get_url is not a simple URL path string.

In this case, Django's resolve_url shortcut might return a URL string that has a URL protocol and hostname already included!

Other Issues

Also, it's unclear whether **kwargs also pass the to keyword argument to resolve_url in our implementation, or just the remainder.

What to Do

  • [ ] Add tests that demonstrate that the described bug exists.
  • [ ] Add a test that demonstrates that to is not included in **kwargs.
  • [ ] Fix the implementation of environment.get_url().

bittner avatar Apr 05 '22 19:04 bittner