-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
v4.ODataModel evaluates preflight requests #3150
Comments
Hello @boghyon , Regards, |
Hi @boghyon, No, OData V4 services are not expected to replicate the client's version header. They are expected to deliver the version they support. See the spec:
|
@UweReeder I see my mistake in my initial issue description.
Should be "responses" not "requests". Sorry about the wrong wording. Edited now. For better understanding of this issue, please refer to the linked discussion in the ODataSamples repo. As this issue is not resolved, would be nice if this issue could be reopened. |
Hi @boghyon, I get a code 500 response when following the link to the samples repo. But nevertheless I think that this is a server issue. According to the OData V4 spec services MUST send the OData-Version header in their responses. Regards, |
Hi @boghyon, unfortunately I am not able to see the issue OData/ODataSamples#29. I get a code 500 response (only for this issue). Nevertheless I think that this is a server issue. The OData V4 spec specifies that a service MUST send the OData-Version header in its responses. Regards, |
@UweReeder Unfortunately, GitHub has an issue with serving some of its pages currently: https://github.community/t/github-issue-responds-with-a-status-of-500/160550. Bad timing! I'll let you know when the issue is accessible again. In the meantime, the issue can be reproduced in https://embed.plnkr.co/IAM5TBfKWaTW8vbg?show=manifest.json,preview&deferRun.
I think this is worth discussing among V4 maintainers here, since the OPTIONS is not really talked about in the OData specification (according to Michael Pizzo from the discussion). Hence my question: "Can the _Requestor 'ignore' responses from the preflight requests or assume missing headers if possible?" i.e. being less strict if the specification doesn't explicitly mention what to do in this case. |
@ThomasChadzelek I just enhanced my last comment with the steps. Hopefully it helps to clarify the issue. |
Hello @boghyon ! @UweReeder and myself had a look in debugger, our code only has access to a few response headers: Best regards, P.S.: I don't think that our code is actually evaluating the preflight response. That should be completely transparent to JS code. From the debugger, it is clear that we are evaluating the actual data response, but have no access to the OData-Version header (although it is present). |
I can confirm that CORS preflight requests are in general not visible to the client code. The browser handles them transparently. |
This is what I also observed. The OPTIONS response had I guess Thank you for the hints! I'll let Michael know about it as soon as the linked issue is accessible again. Closing for now.. |
Just an update for this thread: as @ThomasChadzelek and @UweReeder pointed out, The OData V4 Trippin service now supports CORS. |
Duplicate of #2686 |
The standard TripPin service is getting a CORS support (OData/ODataSamples#29). The responses from preflight (OPTIONS) there, however, don't include e.g.
OData-Version: 4.0
in the header unlike the responses from the real GET requests.Here is a minimal sample: https://embed.plnkr.co/IAM5TBfKWaTW8vbg?show=manifest.json,preview&deferRun
This leads to the error
Expected 'OData-Version' header with value '4.0' but received value 'null'
from the _Requestor:openui5/src/sap.ui.core/src/sap/ui/model/odata/v4/lib/_Requestor.js
Lines 686 to 687 in 3375063
See OData/ODataSamples#29 (comment). Is this a client issue? Can the _Requestor "ignore" responses from the preflight requests or assume missing headers if possible? Or are services supposed to replicate all headers from the real responses (here: GET) to the OPTIONS responses too?
The text was updated successfully, but these errors were encountered: