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

Both request and response methods fail to support external redirects inside controller #768

Closed
gabefair opened this issue Oct 9, 2023 · 6 comments · Fixed by #789
Closed
Labels

Comments

@gabefair
Copy link

gabefair commented Oct 9, 2023

Wishing you the best wishes Joe Mancuso with your Cancer battle!

Describe the bug

This needed feature is not documented.

See: https://stackoverflow.com/q/76300134/635160
and: https://stackoverflow.com/q/77260388/635160

It gets stuck in redirecting

Expected behaviour

The following code should successfully redirect to the external url.

from masonite.views import View
from masonite.controllers import Controller
from masonite.request import Request
from masonite.response import Response


class SubmissionController(Controller):
    def show(self, request: Request, response: Response):
        response.header('Cache-Control', "max-age=0, no-cache, no-store, must-revalidate")
        response.status(307)
        return response.redirect(f'http://msn.com')

OS

Linux

OS version

Ubuntu 22.04

Masonite Version

4.6.1

@gabefair
Copy link
Author

gabefair commented Oct 9, 2023

As a point of reference using the Flask framework this is as easy as

from flask import redirect

def redirect_wrapper(outgoing_url):
  response = redirect(location="o/"+outgoing_url, code=307)
  response.headers['Cache-Control'] = "max-age=0, no-cache, no-store, must-revalidate"
  return response

@josephmancuso
Copy link
Member

@eaguad1337 is this something you can replicate? i couldnt

@josephmancuso
Copy link
Member

@josephmancuso
Copy link
Member

@gabefair what browser was this? chrome right?

@eaguad1337
Copy link
Member

@josephmancuso I confirm this is a bug. I can't confirm why yet but the Location Header is not being set right in response.py line 183.

if location:
    location = add_query_params(location, query_params)
    self.header_bag.add(Header("Location", location))

For any reason, the location var is being emptied and the Location header is being set to an empty string.

I am trying to figure it out to fix it.

@eaguad1337
Copy link
Member

@josephmancuso the error is in str.py, the method add_query_params should return a full url and append query params but it does return only the path.

response.py:183

location = add_query_params(location, query_params)

str.py:79

def add_query_params(url: str, query_params: dict) -> str:
    """Add query params dict to a given url (which can already contain some query parameters)."""
    path_result = parse.urlsplit(url)

    base_url = path_result.path // This is the problem, it uses the path but never concatenates with the domain / host.

    # parse existing query parameters if any
    existing_query_params = dict(parse.parse_qsl(path_result.query))
    all_query_params = {**existing_query_params, **query_params}

    # add query parameters to url if any
    if all_query_params:
        base_url += "?" + parse.urlencode(all_query_params)

    return base_url

See my comment above. In theory, it should be enough concatenating the real base url with the path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants