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

Invalid redirect with callback to absolute URI #10489

Open
fatal10110 opened this issue Feb 1, 2025 · 5 comments
Open

Invalid redirect with callback to absolute URI #10489

fatal10110 opened this issue Feb 1, 2025 · 5 comments
Labels

Comments

@fatal10110
Copy link

fatal10110 commented Feb 1, 2025

What you were expecting:
Im using custom routes, passing redirect property to Create component

<Create
      redirect={(resource, id, data) => `/resourceA/<raId1>/resourceB/<rbId2>/resourceC/${id}`}
      mutationOptions={{ meta: { ... } }}
      resource="resourceC"
    >
...
</Create>

Expected to redirect to the absolute uri returned from the callback /resourceA/1/resourceB/2/resourceC/3

What happened instead:
instead I was redirected to /1/resourceB/2/resourceC/3 , according to the code of redirect function from ra-core

            } else if (typeof redirectTo === 'function') {
                const target: To = redirectTo(resource, id, data);
                const absoluteTarget =
                    typeof target === 'string'
                        ? `${basename}/${target}`
                        : {
                              pathname: `${basename}/${target.pathname}`,
                              ...target,
                          };
                navigate(absoluteTarget, {
                    state: { _scrollToTop: true, ...state },
                });
                return;

the result will be <basename>//resourceA/1/resourceB/2/resourceC/3 the double slash is causing the issue I belive, I would expect the check for leading slash before concatination

Environment

  • React-admin version: 5.4.3
  • Browser: Edge
@fzaninotto
Copy link
Member

I want to be sure about the problem you meet:

  1. the basename has a trailing slash
  2. the redirect target has a leading slash
  3. the resulting URI has a double slash

Is it the right description? If so, I don't see a bug. You should either remove the trailing slash from 1 or the leading slash from 2.

@fatal10110
Copy link
Author

fatal10110 commented Feb 3, 2025

I want to be sure about the problem you meet:

  1. the basename has a trailing slash
  2. the redirect target has a leading slash
  3. the resulting URI has a double slash

Is it the right description? If so, I don't see a bug. You should either remove the trailing slash from 1 or the leading slash from 2.

No, I never set basename, so it does not have the traling slash, u can see the code I posted from ra-core code, it always ads trailing slash to basename, so I can not remove it, as I showed in the coee

pathname: `${basename}/${target.pathname}`,

so this code will not as expected

const redirect = useRedirect()

redirect(() => `/resourceA/1/resourceB/2/resourceC/3`)

@fzaninotto
Copy link
Member

Right, react-admin adds the slash after the basename. But then, why do you include a trailing slash in your redirect URI?

@fatal10110
Copy link
Author

fatal10110 commented Feb 3, 2025

Right, react-admin adds the slash after the basename. But then, why do you include a trailing slash in your redirect URI?

Because your doc says so...

https://marmelab.com/react-admin/useRedirect.html#useredirect

Example 1 from docs

import { useRedirect } from 'react-admin';

const DashboardButton = () => {
    const redirect = useRedirect();
    const handleClick = () => {
        redirect('/dashboard');
    }
    return <button onClick={handleClick}>Dashboard</button>;
};

Example 2 from docs

redirect((resource, id, data) => { 
    return data.hasComments ? '/comments' : '/posts';
}, 'posts', 1, { hasComments: true });

Example 3 from docs

redirect(`/deals/${deal.id}/show`, undefined, undefined, undefined, {
    _scrollToTop: false,
});

p.s I never said I can not do a workaround, but it is just wrong to add trailing slash without checking first if there is slash already

@fzaninotto
Copy link
Member

You're right. Would you like to open a PR to fix useRedirect so that it doesn't add a slash of the target pathname already starts with one?

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

No branches or pull requests

2 participants