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

restapi double parameter is broking code. #1763

Open
folix-01 opened this issue Mar 19, 2024 · 8 comments
Open

restapi double parameter is broking code. #1763

folix-01 opened this issue Mar 19, 2024 · 8 comments

Comments

@folix-01
Copy link
Contributor

For example if u send this type of the request to @search endpoint: @search?expand=breadcrumbs,actions,navroot,navigation,subsite&expand.navigation.depth=2 where u have a double expand parameter(which will be interpreted like a flatten dict and a string) the site is broking at plone.restapi.search.utils:35 when it tries to execute the unflatten_dotted_dict

@stevepiercy
Copy link
Contributor

How should the multiple instances of a parameter be handled?

  • Should they be merged?
  • Last one wins?
  • First one wins?
  • What if values in one instance of the parameter conflict with another?

@folix-01
Copy link
Contributor Author

How should the multiple instances of a parameter be handled?

  • Should they be merged?
  • Last one wins?
  • First one wins?
  • What if values in one instance of the parameter conflict with another?

They are not compatible so as the parameter must be a normal string or a normal flatten dict as I see in plone.restapi(https://github.com/plone/plone.restapi/blob/e62bea2121d929d9c35298e7e5841332bd1dcbc5/src/plone/restapi/serializer/expansion.py#L7C5-L8C1).

I think the BadRequest must be returned to the user in case he uses the both of possible value types.

@stevepiercy
Copy link
Contributor

Agreed. I think that doubled parameters should be rejected. I think that requests should be well-formed, and we should not guess the intent.

@davisagli
Copy link
Sponsor Member

Agreed that the REST API should return a BadRequest response if the input is not in a valid format. @folix-01 Can you show the full stack trace of the error to make it clearer how it is breaking?

@folix-01
Copy link
Contributor Author

@search?expand=breadcrumbs,actions,navroot,navigation,subsite&expand.navigation.depth=2

Hi, sure

Screenshot 2024-03-20 at 09 58 55

@stevepiercy
Copy link
Contributor

@folix-01
Copy link
Contributor Author

@folix-01 https://meta.stackoverflow.com/questions/285551/why-should-i-not-upload-images-of-code-data-errors. Images of text are difficult to read.

Yeah, sorry, my mistake

Traceback (innermost last):
Module ZPublisher.WSGIPublisher, line 181, in transaction_pubevents
Module ZPublisher.WSGIPublisher, line 391, in publish_module
Module ZPublisher.WSGIPublisher, line 285, in publish
Module ZPublisher.mapply, line 98, in mapply
Module ZPublisher.WSGIPublisher, line 68, in call_object
Module plone.rest.service, line 22, in call
Module plone.restapi.services, line 19, in render
Module plone.restapi.services.search.get, line 9, in reply
Module plone.restapi.search.utils, line 35, in unflatten_dotted_dict
Module plone.restapi.search.utils, line 28, in create_or_get
AttributeError: 'str' object has no attribute 'setdefault'

@davisagli
Copy link
Sponsor Member

davisagli commented Mar 21, 2024

I think the search get service should be adjusted to return a 400 response if there's an exception in unflatted_dotted_dict

I'm not sure why you're trying to pass expand parameters to the search service though. They are only used by the content get service. The search service is trying to interpret them as a catalog query.

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

No branches or pull requests

3 participants