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

Not possible to find service when Accept headers with multiple schemes #62

Closed
tiberiuichim opened this issue Apr 25, 2017 · 10 comments
Closed

Comments

@tiberiuichim
Copy link
Contributor

This simple jquery call should work:

  $.ajax(
    {
      url,
      method: 'POST',
      dataType: 'json',
      success: function (resp) { }
    }
  );

The Accept is sent as 'application/json, text/javascript, */*; q=0.01', result of parse_accept_header(accept) is [('application', 'json'), ('text', 'javascript'), ('*', '*')].

Except for cowardly exiting in lookup_service_id:

    if len(media_types) != 1:
        return None

IMHO, it should loop over the result of media_types and only return None if no services are found.

@tisto
Copy link
Member

tisto commented Apr 25, 2017

@tiberiuichim plone.rest is a low level framework to create custom REST endpoints, not an out-of-the box solution that allows you to do POST calls via jQuery. You might want to look into plone.restapi.

@tisto tisto closed this as completed Apr 25, 2017
@tisto
Copy link
Member

tisto commented Apr 25, 2017

Feel free to re-open if I misunderstood your report.

@tiberiuichim
Copy link
Contributor Author

I'm using plone.restapi.

@tiberiuichim tiberiuichim reopened this Apr 25, 2017
@tiberiuichim
Copy link
Contributor Author

My concern is that this common use case, of doing a call with jquery is not supported. I know, it is possible to set the exact headers in jquery.ajax(), but I think the default should be supported OOTB

@tiberiuichim
Copy link
Contributor Author

We can also go strictly by protocol definitions. "My client" says that it accepts any of application/json, text/javascript or really really any response (based on the wildcard), while plone.rest will refuse to return a proper response because it wants that my client only accept application/json, where "proper" is the appropriate service for the application/json encoding. This is wrong, from my perspective.

@tisto
Copy link
Member

tisto commented Apr 26, 2017

@tiberiuichim then please provide a curl/wget/httpie example of even better a python test to prove your point.

@tisto
Copy link
Member

tisto commented Apr 27, 2017

@tiberiuichim sorry for misunderstanding your bug report at first. The problem is that the underlying existing implementation in the zpublisher is very complex. Therefore we decided to implement content negotiation in the simplistic way we did.

What you describe is certainly desirable, but we will not be able to implement that anytime soon. The amount of work that would be necessary to fix does not rationalize what we gain from it. Therefore I will close the issue. Feel free to re-open when you or somebody else wants to tackle this. Though, I would not advise doing so.

What you describe is certainly desirable, but we will not be able to implement that anytime soon. The amount of work that would be necessary to fix does not rationalize what we gain from it. Therefore I will close the issue. Feel free to re-open when you or somebody else wants to tackle this. Though, I would not advise doing so.

@tisto tisto closed this as completed Apr 27, 2017
@tiberiuichim
Copy link
Contributor Author

Maybe I'm missing something, but wouldn't something like this be enough?

diff --git a/src/plone/rest/negotiation.py b/src/plone/rest/negotiation.py
index ffc6231..e0718ae 100644
--- a/src/plone/rest/negotiation.py
+++ b/src/plone/rest/negotiation.py
@@ -24,17 +24,15 @@ def lookup_service_id(method, accept):
        negotiation.
     """
     media_types = parse_accept_header(accept)
-    if len(media_types) != 1:
-        return None
-    type_, subtype = media_types[0]
-    types = _services.get(method, {})
-    subtypes = types.get(type_, {})
-    if subtype in subtypes:
-        return subtypes[subtype]
-    elif '*' in subtypes:
-        return subtypes['*']
-    if '*' in types:
-        return types['*']['*']
+    for type_, subtype in media_types:
+        types = _services.get(method, {})
+        subtypes = types.get(type_, {})
+        if subtype in subtypes:
+            return subtypes[subtype]
+        elif '*' in subtypes:
+            return subtypes['*']
+        if '*' in types:
+            return types['*']['*']
     return None

@buchi
Copy link
Member

buchi commented May 1, 2017

@tiberiuichim the check for a single media type in the Accept header is intentional. We use that to distinguish between regular requests and REST API requests. With your proposed change every regular browser request would go to the REST API endpoint as browsers always include */* in their Accept header.

@petri
Copy link
Member

petri commented Oct 5, 2021

This use case may actually become supported if/when #105 is merged.

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

4 participants