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

Slash in key lookup breaks URI parser #735

Open
shubhamguptacal opened this issue Dec 12, 2016 · 10 comments
Open

Slash in key lookup breaks URI parser #735

shubhamguptacal opened this issue Dec 12, 2016 · 10 comments

Comments

@shubhamguptacal
Copy link

shubhamguptacal commented Dec 12, 2016

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
Copy link

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
Copy link
Member

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
Copy link

flieks commented Sep 13, 2018

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

@mikepizzo mikepizzo reopened this Sep 13, 2018
@mikepizzo
Copy link
Member

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

@flieks
Copy link

flieks commented Sep 13, 2018

@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
Copy link

josip8 commented Jul 28, 2020

can confirm it is still not working

  • asp.net core 3.1

  • Microsoft.AspNetCore.OData (7.3.0)

@ehsansccm
Copy link

I confirm that the issue still reproduces on 7.8.3

URI Parser breaks with this error:

Bad Request - Error in query syntax

@davidmorissette
Copy link

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

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

@mPisano
Copy link

mPisano commented Jul 29, 2021

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
Copy link

fandecheng commented Aug 17, 2022

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.

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

No branches or pull requests

10 participants