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

Should plone.restapi follow redirects? #181

Closed
erral opened this issue Jan 4, 2017 · 2 comments
Closed

Should plone.restapi follow redirects? #181

erral opened this issue Jan 4, 2017 · 2 comments

Comments

@erral
Copy link
Sponsor Member

erral commented Jan 4, 2017

When for the public site a view that does a redirect is set, plone.restapi doesn't follow the redirect, but should that be the proper behavior?

Use case:

  • You have a multilingual site (either using LinguaPlone or plone.app.multilingual) with 2 languages: en and es.
  • Both products install a language switcher view on the root folder of the Plone site to do the proper language negotiation and redirect the end user to the proper language root folder (either /en or /es in this case).
  • If you browse the root of your site with a browser, or using wget, you will see that the language switcher returns a 301 redirect; but in plone.restapi the site root serializer comes into play and returns the contents of the Plone site https://github.com/plone/plone.restapi/blob/master/src/plone/restapi/serializer/site.py#L21

I guess (totally untested), that the same happens for any other content on the site: if it has a default page that creates a redirect, the contents themselves are serialized and the redirect is not followed.

But as I stated above: is to follow the redirect a proper behaviour for plone.restapi?

@erral
Copy link
Sponsor Member Author

erral commented Mar 5, 2017

plone.app.redirector redirects are handled using a browser view inside Plone's own default_error_message.pt: https://github.com/plone/Products.CMFPlone/blob/5.0.x/Products/CMFPlone/skins/plone_templates/default_error_message.pt#L30 and https://github.com/plone/plone.app.redirector/blob/master/plone/app/redirector/browser.py#L26)

To check at least for plone.app.redirector redirects in plone.restapi we should do the same but in plone.rest, where we currently check for a NotFound exception and return an error (https://github.com/plone/plone.rest/blob/master/src/plone/rest/errors.py#L39)

@lukasgraf
Copy link
Member

I'm considering tackling this on the upcoming Beethoven sprint.

I've been thinking about this a while, and also talking to co-workers as to what behavior they would expect from a Plone API, and at this point I'm quite convinced that plone.restapi should return redirects, the same way a regular Plone site does.

The client then has all the options to deal with it:

  • Treat it like a 404 (that's basically what the server does now for the client)
  • Follow the redirect

So in the case of GET requests, I'm pretty sure we want this. In the case of other request methods, it's slightly less clear. Browser behavior for POST requests that get answered with a 30x isn't well defined - implementations seem to vary.

However, in the case of an API client I would argue that this is a custom and well controlled enough application that we can simply respond with redirects too, and it again is up to the clients how they handle this for something like POST. Maybe they'll want to POST again to the resource indicated in the Location header, maybe not.

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

No branches or pull requests

3 participants