SAP / python-pyodata

Enterprise-ready Python OData client
Apache License 2.0
224 stars 93 forks source link

Incorrect escaping of path #282

Open frij-aws opened 2 months ago

frij-aws commented 2 months ago

Greetings,

pyodata does not correctly escape the path when an entity key contains a /. For example SAP's OData service for Software Components retrieves an SC called ZFOO with the URL:

https://hostname/sap/opu/odata/sap/A4C_A2G_GHA_SC/SoftwareComponents%28%27ZFOO%27%29

But if I try to get an SC called /BAZ/FOO the URL is malformed as:

https://hostname/sap/opu/odata/sap/A4C_A2G_GHA_SC/SoftwareComponents%28%27/BAZ/FOO%27%29

but the / characters ought to be escaped as

https://hostname/sap/opu/odata/sap/A4C_A2G_GHA_SC/SoftwareComponents%28%27%2FBAZ%2FFOO%27%29

Looking at the pyodata code I can see that EntityGetRequest.get_path() is calling quote() which by default considers safe='/'. It seems to me it should override safe='' so that / characters are escaped. I am new to the pyodata code so I'm not sure if this issue might also lurk on operations other than EntityGetRequest. I'll appreciate any feedback or expansion on my analysis.

phanak-sap commented 1 month ago

Hi @frij-aws.

This is interesting case, so far we did not encounter such interesting entity name.I tried to contact you first using internal SAP channels, since there was not mentioned of what service this is or $metadata document provided. But with the PR it is clearer what the intentions are.

Just to confirm, it is a case of having in metadata something like <EntityType Name="BAZ/FOO">. Correct?

I have first wanted to check if this is a case of some specific odata service(so it should be under /vendor) or generic odata protocol. Odata v2 IMHO does not specify this exactly, resp. I was not able to find it on odata.org. But in Odata V4 specification is referenced RFC 3987 - Internationalized Resource Identifiers (IRIs) for Entity ID and v4 vs v2 is mostly backward compatible. This is definitely not in the list of breaking changes for V4, so I would assume it is valid for generic odata V2as well.

Run into this:

The entity-id MUST be an IRI as defined in [RFC3987] and MAY be expressed in payloads, headers, and URLs as a relative reference as appropriate.

https://docs.oasis-open.org/odata/odata/v4.01/csprd06/part1-protocol/odata-v4.01-csprd06-part1-protocol.html#sec_EntityIdsandEntityReferences

So I see this is as a valid and generic bug for the service module.