Http data plane only allows query parameters with unique keys

[arnoweiss]

Bug Report

Describe the Bug

It's a bug in how the Http data plane proxies the query parameters of incoming requests.

Expected Behavior

Http data planes can be configured in such a manner that they forward query parameters provided by the consumer. In that case a call GET https://provider-data.plane?param=value will be proxied as GET https://provider-back.end?param=value. This also works if multiple distinct keys are provided for separate query parameters. If query parameters are of type array, transmitting the entries of said array as separate query parameters with equivalent keys is a common practice, yielding requests like GET https://provider-back.end?param=value1&param=value2&param=value3.

Observed Behavior

The http data plane currently doesn't proxy these parameters properly. A request GET https://provider-data.plane?param=value1&param=value2&param=value3 will be proxied as GET https://provider-data.plane?param=value1.

Steps to Reproduce

1. (Provider) setup arbitrary asset with "type":"HttpData" and "proxyQueryParams":"true"
2. (Provider) publish via contract definition
3. (Consumer) find asset in catalog
4. (Consumer) negotiate, initiate transfer process, receive EDR
5. (Consumer) call GET https://provider-data.plane?param=value1&param=value2&param=value3
6. (Provider) Observe resulting request in backend

Context Information

Observed with tractusx-edc 0.6.0 (thus eclipse-edc 0.5.1). This was first raised in the EDC matrix channel by @sachinargade123.

[ciprianherciu]

Hello @arnoweiss, could You tell me if it is planned to solve the issues in 24.05 release?

[ciprianherciu]

Hello @stefan-ettl, is it planned to solve this issue for the 24.05 release?

[paullatzelsperger]

@arnoweiss and @ciprianherciu not exactly sure this really is a bug, as RFC 3986, section 3.4 Query is kinda vague about it. I would thus classify it as feature request.

What is your use case for having multiple identical query params? To me it seems that attempting to send information as key-to-multi-value datastructure in the request URL is a bit of a code smell. Since it is neigher explicitly forbidden or prescribed by the standard, I would not rely on it. Any proxy, gateway, loadbalancer, etc can behave in an unexpected way. Sending a JSON array in the body of a POST request would be more adequate IMO.

@ciprianherciu this is the Eclipse Dataspace Components project, any downstream releases such as Tractus-X 24.05 are neither relevant here, nor can you expect committers here to be aware of them.

I'm not convinced there is significant merit for this to be implemented, but if so, it would be subject to the EDC project triage and priorization process.

Also mentioning Tractus-X Matrix chatrooms is not really helpful, as not every EDC committer has access (or reason to attend) there.

[arnoweiss]

This issue becomes relevant when a client is attempting to access a Digital Twin Registry (DTR) via the http data plane.

sequenceDiagram
    participant C as Consumer
    participant CCP as Consumer Control Plane
    participant PCP as Provider Control Plane
    participant PDP as Provider Data Plane
    participant DTR as Digital Twin Registry
    CCP->PCP: catalog, negotiate, trigger transfer
    PCP -->>C: EDR
    C->>PDP: /lookup/shells?assetIds=foo&assetIds=bar
    PDP ->>DTR: /lookup/shells?assetIds=foo
    DTR -->> PDP: response (only for assetIds=foo)
    PDP -->> C: response (only for assetIds=foo)

Loading

A patch to the Asset-Administration-Shell-Spec has clarified that the query parameter of the operation GetAllAssetAdministrationShellIdsByAssetLink is an array of encoded objects. The reference implementation eclipse-tractusx/sldt-digital-twin-registry has implemented it as described in the initial post. Thus, whenever a client executes a request to a DTR with mutliple assetIds parameters, the http data plane removes all but one. I don't want to comment on the quality of the AAS spec here - either way, clients and servers rely on this mechanism until there's a more reliable and disambiguous mechanism.

[jimmarino]

Why can't this be corrected in the SDTR? Given the ambiguity associated with this in the referenced RFC, the fact that other data planes may not support it, and the fact that there are better ways to represent this as a RESTFul HTTP request, the proper solution is probably to address this in AAS and SDTR.

[lgblaumeiser]

As @arnoweiss mentioned, the /lookup/shells endpoint of the DTR is specified in this way in the AAS standard linked in his comment above. So, a solution in the DTR is not a simple implementation detail but would require a change in the standards.

I have also checked the behavior of the Swagger Tooling for an OpenAPI spec, and it translates a query parameter of type array[string] into exactly this sequence of a multi usage of the same key in the query param string. So, although the RFC might be ambiguous, this seems to be expected standard behavior of a rest GET endpoint.

And finally, in general, in the role of a proxy, it is non-expected behavior, that the semantics of the request are changed by removing parts of the query that needs to be forwarded to the proxied system.

As this issue basically prevents a proper usage of the standardized DTR endpoint, we at Cofinity-X consider this as an urgent issue to be solved for the upcoming release.

[thomas-henn]

I completely agree to @arnoweiss, @ciprianherciu, and @lgblaumeiser.
The DTR is, as aligned in the whole Catena-X activities, compliant to the specifications of the Asset Administration Shell of the IDTA and ALL use cases, which rely on Digital Twins are blocked by this bug.

How can we support here @jimmarino, @paullatzelsperger to get the bug fixed within Release 24.05.
Please let us know.

[jimmarino]

This is not a bug but an enhancement. We will need to triage and prioritize it according to standard procedures. We will discuss this during our next committer's meeting.

[paullatzelsperger]

@lgblaumeiser I understand, and the technical functionality is not the issue, since the fix would be trivial to do. It's only a couple of lines.

I do reject the notion that it is a bug, simply because the aforementioned RFC does not classify it as one.

All issues in EDC are subject to the committers' triage process and decision, and will be prioritized accordingly. It is understood that you think it is urgent and you would like it in the next EDC version, and that will be considered. I do want to point out though, that EDC version releases are not coupled to releases of downstream projects, such as Tractus-X.

@thomas-henn so there really is nothing to support here, all that is required is adherence to proper procedure.

[BirgitBoss]

I am very surprised about the completely unexpected behavior of the EDC of cutting of parameters: the handling of multiple parameters is standard http/REST behavior and there is not question that it must be supported. Since the complete Catena-X release is endangered by this bug-feature discussion: please just do it. It does not matter whether as a bug or as a feature with highest priority if you prefer. But please do not postpone the implementation by this ping-pong discussion.

P.S. For me it is clearly a bug.

[jimmarino]

@BirgitBoss Please read the comment by @paullatzelsperger. This feature request must be triaged and our procedures followed. This is not a question of shifting blame or responsibility but how EDC as a project operates.

That does not mean the feature will not be implemented. In fact, the work is minimal. Please respect our processes and allow the committers to do their jobs.

[BirgitBoss]

I did read it: I assume there is another process for bugs. So I do not understand why you are not accepting that this is a bug and follow the faster bug-process.

[paullatzelsperger]

@BirgitBoss the only people who get to decide the class of issue (bug vs feature request) and its associated priority and thus: release schedule, are the committers of EDC, as per Eclipse governance rules.

In the same spirit I think this discussion is not a waste of time, and indeed provides great value, because there clearly is a gap in the understanding of open-source procedures.

[edit]: no-one disputes the fact that supporting this is probably a good idea, only that using it is bad design and that procedures are important.

[paullatzelsperger]

I did read it: I assume there is another process for bugs. So I do not understand why you are not accepting that this is a bug and follow the faster bug-process.

Because classifying it is a group decision of the committers, and we have procedures for that. Whether you agree to them or the outcome is immaterial.

[lgblaumeiser]

Thanks, @paullatzelsperger @jimmarino for taking this up. It was my only intention, that it is considered with the appropriate process and priority. I think, everything is said and lets proceed according to the process.

Btw, there is imho no value in discussing the classification of the issue. It is a requirement that is currently not met by the EDC and the urgency has been made clear.

[paullatzelsperger]

Update from today's committer meeting:

since the fix is trivial, we will include this in the Milestone 14 release (EDC Version 0.6.0), which is scheduled to be released on March 27.

Further, the requested behaviour will only be implemented in the public API of the DataPlane Signaling feature pack, it won't be backported to the legacy data plane API.

For the people involved in this thread that won't matter, because Tractus-X will only have the new data plane API.

Note: Whether Tractus-X 24.05 (= Tractus-X EDC 0.7.x) will use EDC 0.6.0 or a subsequent bugfix release is yet to be determined.