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

More rigorous definition of request references #1917

Merged
merged 17 commits into from
May 29, 2024
Merged

More rigorous definition of request references #1917

merged 17 commits into from
May 29, 2024

Conversation

HeikoTheissen
Copy link
Contributor

No description provided.

@HeikoTheissen HeikoTheissen linked an issue Apr 4, 2024 that may be closed by this pull request
@ralfhandl ralfhandl assigned ralfhandl and HeikoTheissen and unassigned ralfhandl May 8, 2024
ralfhandl
ralfhandl previously approved these changes May 13, 2024
@ralfhandl
Copy link
Contributor

Homework: cross-check our resolution rules against https://www.rfc-editor.org/rfc/rfc3986#section-5, especially for multi-part responses.

@HeikoTheissen
Copy link
Contributor Author

HeikoTheissen commented May 23, 2024

Here's my argumentation:

"5.1.2. Base URI from the Encapsulating Entity" takes precedence over "5.1.3. Base URI from the Retrieval URI". In the case of an individual response within a $batch response the encapsulating entity is the $batch response, but this has no base URL for lack of a Content-Location header. Therefore 5.1.2 is not applicable in this case and 5.1.3 becomes relevant.

We should state in [OData-Protocol, section 11.7] that the "URI used to retrieve the representation" of an individual response within a $batch response is the corresponding individual request URL, in both multipart and JSON $batch.

I found a problem:

Section 11.7.7.1 states

The request URL of individual requests within a batch request or change set can use one of the following three formats:
...

  • Absolute resource path and separate Host header

Example 102:

PATCH /path/service/People(1) HTTP/1.1
Host: myserver.mydomain.org:1234
...

But path and Host header are not sufficient to determine the protocol (http:) of the request URL.

Copy link
Contributor

@ralfhandl ralfhandl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, one minor nit

ralfhandl
ralfhandl previously approved these changes May 23, 2024
@HeikoTheissen
Copy link
Contributor Author

But path and Host header are not sufficient to determine the protocol (http:) of the request URL.

What about this problem?

@ralfhandl
Copy link
Contributor

But path and Host header are not sufficient to determine the protocol (http:) of the request URL.

What about this problem?

The scheme is not necessary inside the batch request because the HTTP message to be routed over the network is the whole batch request, and its scheme has already been used to establish a secured or unsecured connection.

@HeikoTheissen
Copy link
Contributor Author

We need an absolute request URL which must serve as the base URI of a relative Location URL. And if the scheme is unclear, we cannot construct one in Example 102.

@ralfhandl
Copy link
Contributor

We need an absolute request URL which must serve as the base URI of a relative Location URL. And if the scheme is unclear, we cannot construct one in Example 102.

Don't we have that by taking the batch request's scheme, the Host header value, and the absolute path from the request line?

@HeikoTheissen
Copy link
Contributor Author

Don't we have that by taking the batch request's scheme, the Host header value, and the absolute path from the request line?

If we take the scheme from request, why not the host as well? Why do we need the Host header then?

@ralfhandl
Copy link
Contributor

ralfhandl commented May 23, 2024

Why do we need the Host header then?

Because that's HTTP/1.1 message format, and it is valid to just wrap those into a multipart body.

We can of course debate whether the Host header has any meaning here, or ask how existing implementations use this second form.

The third form with relative paths in the request line needs to be resolved against the batch URL anyway, which essentially means prefixing with the service root URL.

@ralfhandl ralfhandl merged commit ac48ba0 into main May 29, 2024
1 check passed
@ralfhandl ralfhandl deleted the $1-references branch May 29, 2024 16:32
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

Successfully merging this pull request may close these issues.

More rigorous definition of $<request identifier> references
2 participants