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

Redirects don't always return absolute URL #384

Closed
jams2 opened this issue Feb 12, 2024 · 3 comments
Closed

Redirects don't always return absolute URL #384

jams2 opened this issue Feb 12, 2024 · 3 comments

Comments

@jams2
Copy link
Contributor

jams2 commented Feb 12, 2024

Given the query:

fragment Redirect on Redirect {
  isPermanent
  newUrl
  oldPath
  oldUrl
  page {
    id
  }
  site {
    id
    hostname
    port
    siteName
    rootPage {
      id
    }
    isDefaultSite
  }
}

query MyQuery {
  redirects {
    ...Redirect
  }
}

and data:

 id | old_path | is_permanent |        redirect_link         | redirect_page_id | site_id | automatically_created |          created_at           | redirect_page_route_path 
----+----------+--------------+------------------------------+------------------+---------+-----------------------+-------------------------------+--------------------------
  2 | /foo/bar | t            | https://example.com/foo/baz/ |                  |       2 | f                     | 2024-02-01 10:59:52.64715+00  | 
  1 | /quux    | t            |                              |                3 |         | f                     | 2024-02-01 10:59:18.243172+00 | 

I get results:

{
  "data": {
    "redirects": [
      {
        "isPermanent": true,
        "newUrl": "https://example.com/foo/baz/",
        "oldPath": "/foo/bar",
        "oldUrl": "http://localhost:3000/foo/bar",
        "page": null,
        "site": {
          "hostname": "localhost",
          "id": "2",
          "isDefaultSite": true,
          "port": 3000,
          "rootPage": {
            "id": "3"
          },
          "siteName": "Localhost"
        }
      },
      {
        "isPermanent": true,
        "newUrl": "/",
        "oldPath": "/quux",
        "oldUrl": "http://localhost:3000/quux",
        "page": {
          "id": "3"
        },
        "site": {
          "hostname": "localhost",
          "id": "2",
          "isDefaultSite": true,
          "port": 3000,
          "rootPage": {
            "id": "3"
          },
          "siteName": "Localhost"
        }
      }
    ]
  }
}

Note the new URL for /quux is relative.

@zerolab
Copy link
Member

zerolab commented Feb 12, 2024

Possibly related to the changes in #380

@jams2
Copy link
Contributor Author

jams2 commented Feb 19, 2024

Having thought about this some more, it seems to me that it should be OK for oldUrl to be relative - taking into account that a redirect may be "from all sites". However, it would make sense for newUrl to always be absolute - users can either select a Page to redirect to, which must belong to a site to be routable, or enter a URL, which validation prevents from being relative.

URLField is really just a CharField with validation, so it's possible that junk values exist in the database for Site.redirect_link, but that seems unlikely under normal circumstances.

@jams2
Copy link
Contributor Author

jams2 commented May 10, 2024

Closing this, I think the current behaviour is correct. See #391 (comment)

@jams2 jams2 closed this as completed May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants