OData / odata.net

ODataLib: Open Data Protocol - .NET Libraries and Frameworks
https://docs.microsoft.com/odata
Other
685 stars 349 forks source link

Slash in key lookup breaks URI parser #735

Open shubhamguptacal opened 7 years ago

shubhamguptacal commented 7 years ago

In the file odata.net/src/Microsoft.OData.Core/UriParser/Parsers/UriPathParser.cs,

I see the following comment -

// COMPAT 29: Slash in key lookup breaks URI parser // TODO: The code below has a bug that / in the named values will be considered a segment separator // so for example /Customers('abc/pqr') is treated as two segments, which is wrong.

I just wanted to check when this issue will be fixed.

markdstafford commented 7 years ago

This seems like it is by design, Mike Pizzo to verify. The comment is actually what appears to be wrong - the slash in the key should be URL encoded, i.e. /Customers('abc%2Fpqr').

mikepizzo commented 7 years ago

Forward slashes within the URL (including within a quoted key value) MUST be URL-encoded.

See http://docs.oasis-open.org/odata/odata/v4.0/errata03/os/complete/part2-url-conventions/odata-v4.0-errata03-os-part2-url-conventions-complete.html#_Toc453752335.

The spec includes, as an example of an invalid URL: http://host/service/Categories('Smartphone/Tablet') 

And calls out the correct format: http://host/service/Categories('Smartphone%2FTablet')

flieks commented 6 years ago

@mikepizzo http://host/service/Categories('Smartphone%2FTablet') doesn't work anymore in .NET Core

mikepizzo commented 6 years ago

@flieks; can you clarify what version of the libraries you are using? Can you provide a simple repo?

flieks commented 6 years ago

@mikepizzo we are using Microsoft.AspNetCore.OData (7.0.1) Microsoft.NETCore.App 2.1.0

our startup code:

public void Configure(
            IApplicationBuilder app,
            IHostingEnvironment env)
{
            var builder = new ODataConventionModelBuilder();
            builder.EnableLowerCamelCase();

           app.UseAuthentication();
            app.UseMvc(options =>
            {
                options.Select().Expand().Filter().OrderBy().MaxTop(null).Count();
                options.MapODataServiceRoute("odata", "odata", builder.GetEdmModel());
            });
}
public void ConfigureServices(IServiceCollection services)
{
            services.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_1);
            services.AddOData();
}

the controller:

   /// <summary>
        /// Gets all <see cref="SalesPart"/> entities from the database.
        /// </summary>
        /// <returns></returns>
        [EnableQuery(MaxExpansionDepth = 10)]
        public new ActionResult Get()
        {
            return base.Get();
        }

        /// <summary>
        /// Gets a single <see cref="SalesPart"/> entity from the database.
        /// </summary>
        /// <param name="key"></param>
        /// <returns></returns>
        [EnableQuery(MaxExpansionDepth = 10)]
        public new ActionResult Get([FromODataUri] string key)
        {
            return base.Get(key);
        }

        /// <summary>
        /// Gets the default image for the SalesPart
        /// </summary>
        /// <param name="key"></param>
        /// <returns></returns>
        [HttpGet]
        public async Task<IActionResult> DefaultImage([FromODataUri] string key)
        {

        }

it's coming inside DefaultImage when doing odata/customers('1')/defaultimage but not with odata/customers('1%2Fa')/defaultimage

josip8 commented 4 years ago

can confirm it is still not working

ehsansccm commented 3 years ago

I confirm that the issue still reproduces on 7.8.3

URI Parser breaks with this error:

Bad Request - Error in query syntax

davidmorissette commented 3 years ago

Until this bug is fixed, I am using this syntax to call my function:

odata/customers(Key=@Key)?@Key='1%2Fa'

mPisano commented 3 years ago

I have modified UriPathParser.cs for Framework 4x https://github.com/OData/odata.net/blob/master/src/Microsoft.OData.Core/UriParser/Parsers/UriPathParser.cs

Theory is reencode the slashes that are between single quotes and then split on the remaining slashes. The already present segments.Add(Uri.UnescapeDataString(segment)); will unencode the final array passed out

I've removed: numberOfSegmentsToSkip = serviceBaseUri.AbsolutePath.Split('/').Length - 1; string[] uriSegments = uri.AbsolutePath.Split('/');

And added: using System.Text.RegularExpressions; ...

numberOfSegmentsToSkip = serviceBaseUri.AbsolutePath.Split('/').Length - 1;

            string uriPath = uri.AbsolutePath;
            foreach (Match item in Regex.Matches(uriPath, @"\'([^\']+)\'"))
            {
                var fragment = item.Value;
                if (fragment.Contains("/"))
                {
                    string newfrag = fragment.Replace("/", "%2F");
                    uriPath = uriPath.Replace(fragment, newfrag);
                }
            }
            string[] uriSegments = uriPath.Split('/');

So far testing is good HTH Mike

fandecheng commented 2 years ago

On Microsoft.AspNetCore.App 3.1.28 and Microsoft.AspNetCore.OData 8.0.9, tested: example URL: https://localhost/myApp/Books(Title='a%2Fb%25%20%22%29''%21%23%24%25%26%28%29%2a%2b%2c%2d%2e%2f%30%3b%3c%3d%3e%3f%40%41%5a%5b%5c%5d%5e%5f%60%61%7a%7b%7c%7d%7e',Year='2022')

C# parameter: [HttpGet("Books(Title={path},Year={logicalId})")] ... public async Task Get( [FromODataUri] string title, [FromODataUri] string year)

What I got in the title parameter in C# (replacing @ with @ to avoid markdown misinterpretation): a%2Fb% ")'!#$%&()*+,-.%2f0;<=>?@AZ[\]^_`az{|}~

This means: 1. Route matching works as expected; 2. Except %2F, all other escape sequences are unescaped.

But I still think, for application programmers, it would be nice to have all characters unescaped in the parameter. Otherwise, there is no perfect way to only unescape %2f. For example, if I replace all %2f with /, then if the source URL has %252F, then it would appear as %2f in the parameter as well, but it apparently should not be unescaped, because the percent sign is a literal character.