Inconsistent encoding of odata.id on server and client

[uffelauesen]

Calculation of self links is encoded on client but unencoded on server.

Assemblies affected

OData .Net lib 7.2 hosted under AspNetCore OData 8.2

Reproduce steps

I have an OData V4 service implemented with AspNetCore OData and ODataLib.
I have a model with an entity called CustomLabel.
CustomLabel has a single string type key property called ID.
I have a CustomLabel instance with ID = "AA,TY".
Serializing this entity with odata.matadata=full gives unencoded odata.id and odata.editLink like .../CustomLabels('AA,TY').
If odata.metadata=minimal the odata.id and odata.editLink are constructed in OData Client from the metadata specification, but here they endup encoded like .../CustomLabels('AA%2CTY').
This gives problems with DataServiceContext.Entites when the same entity are returned from multiple requests with diferent odata.metadata settings and the same entity is not correctly matched against the already tracked entity and it will endup with multiple entries in the tracked entity collection.

Should the id and editlink be encoded or not?
What is correct and what is incorrect?
Is it a bug in OData.Client, OData.Core or in AspNetCore.OData?

Expected result

The id, editLink (selfLinks) are the same regardles if they are picked op from the payload or generated on the client from metadata.

Actual result

The OData Client sees the same entity as 2 different entities as their ids are not exactly the same. Check the last 2 EntityDescriptor entries in this context.Entities watch...

Additional detail

On OData V3 (using WCF Data Services) the id and editLinks returned from server were encoded like .../CustomLabels('AA%2CTY')..

[habbes]

This inconsistency is likely a bug. Curious, does the server not encode the url at all, or it encodes a different set of characters than the client? (asking because I don't think we consider a comma as a special character that has to be escaped).

[uffelauesen]

Hi Clément
It doesn't encode key segments at all.
In AspNetCoreOData's ODataResourceSerializer.CreateResource() you have the following...

Microsoft.AspNetCore.OData.Edm.NavigationSourceLinkBuilderAnnotation navigationSourceLinkBuilder = EdmModelLinkBuilderExtensions.GetNavigationSourceLinkBuilder(model, resourceContext.NavigationSource);
EntitySelfLinks entitySelfLinks = navigationSourceLinkBuilder.BuildEntitySelfLinks(resourceContext, resourceContext.SerializerContext.MetadataLevel);
if (entitySelfLinks.IdLink != null)
{
    oDataResource.Id = entitySelfLinks.IdLink;
}

Here BuildEntitySelfLinks calls LinkGeneratorHelpers.CreateODataLink() that in turn calls ODataPathExtensions.GetPathString() that uses ODataPathSegmentHandler to walk the segments and constructing the path. ODataPathSegmentHandler calls ODataUriUtils.ConvertToUriLiteral(). In the end all this ends up calling ODataUriConversionUtils.ConvertToUriPrimitiveLiteral(), that looks like this...

        /// <summary>
        /// Converts a primitive to a string for use in a Url.
        /// </summary>
        /// <param name="value">Value to convert.</param>
        /// <param name="version">OData version to be compliant with.</param>
        /// <returns>A string representation of <paramref name="value"/> to be added to a Url.</returns>
        internal static string ConvertToUriPrimitiveLiteral(object value, ODataVersion version)
        {
            ExceptionUtils.CheckArgumentNotNull(value, "value");

            // TODO: Differences between Astoria and ODL's Uri literals
            /* This should have the same behavior of Astoria with these differences:
             * 1) Cannot handle the System.Data.Linq.Binary type
             * 2) Cannot handle the System.Data.Linq.XElement type
             * 3) Astoria does not put a 'd' or 'D' on double values
             */

            // for legacy backwards compatibility reasons, use the formatter which does not URL-encode the resulting string.
            return LiteralFormatter.ForConstantsWithoutEncoding.Format(value);
        }

If I override CreateResource and inject my own ODataResourceSerializer, I can fix the issue with the following workarond that will generate the id encoded correctly...

        public override ODataResource CreateResource(SelectExpandNode selectExpandNode, ResourceContext resourceContext)
        {
            ODataResource odataResource = base.CreateResource(selectExpandNode, resourceContext);
            if (odataResource.Id != null)
            {
                // Workaround for https://github.com/OData/odata.net/issues/2857

                var feature = resourceContext.Request.ODataFeature();
                Uri baseUri = new Uri(feature.BaseAddress + '/', UriKind.Absolute);
                var parser = new ODataUriParser(
                    resourceContext.EdmModel,
                    baseUri,
                    odataResource.Id,
                    resourceContext.Request.GetRouteServices());

                ODataPath path = parser.ParsePath();
                odataResource.Id = new Uri(baseUri, path.ToResourcePathString(ODataUrlKeyDelimiter.Parentheses));
            }
        }

[uffelauesen]

Moved this issue to AspNetCoreOData repos: OData/AspNetCoreOData#1390

[uffelauesen]

This inconsistency is likely a bug. Curious, does the server not encode the url at all, or it encodes a different set of characters than the client? (asking because I don't think we consider a comma as a special character that has to be escaped).

Only slash / gets encoded from a string key in odata links and in Location header. This is done in AspNet Core OData. As said I moved this report to that repos.