plone / plone.restapi

RESTful API for Plone.
http://plonerestapi.readthedocs.org/
84 stars 73 forks source link

restapi double parameter is broking code. #1763

Open folix-01 opened 3 months ago

folix-01 commented 3 months ago

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 commented 3 months ago

How should the multiple instances of a parameter be handled?

folix-01 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

Hi, sure

Screenshot 2024-03-20 at 09 58 55
stevepiercy commented 3 months ago

@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.

folix-01 commented 3 months ago

@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 commented 3 months ago

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.