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

Specifying '.{format?}' in a route template throws exception in some cases #3553

Closed
kichalla opened this Issue Nov 13, 2015 · 10 comments

Comments

Projects
None yet
7 participants
@kichalla
Member

kichalla commented Nov 13, 2015

Following setup works for the following cases:
/api/values
/api/values.xml
/api/values/10
/api/values/10.xml

[Route("api/[controller]")]
public class ValuesController : Controller
{
    [FormatFilter]
    [HttpGet]
    [HttpGet("/api/[controller].{format}")] 
    public IEnumerable<string> Get()
    {
        return new string[] { "value1", "value2" };
    }

    [FormatFilter]
    [HttpGet("{id}.{format?}")]
    public string Get(int id)
    {
        return "value";
    }

But when I try just having one route like [HttpGet("/api/[controller].{format?}")] on the first action I got the following exception:

InvalidOperationException: The following errors occurred with attribute routing information:

For action: 'WebApplication51.Controllers.ValuesController.Get'
Error: In the segment 'Values.{format?}', the optional parameter 'format' is preceded by an invalid segment 'Values.'. Only a period (.) can precede an optional parameter.
Parameter name: routeTemplate

Note that having the ‘.{format?}’ on the second action does not throw this error but it thrown for the first action.

Discussed with @rynowak

@kichalla kichalla added the bug label Nov 13, 2015

@danroth27

This comment has been minimized.

Show comment
Hide comment
@danroth27

danroth27 Nov 16, 2015

Member

Can we also consider adding the FormatFilter globally when the format mappings are setup?

Member

danroth27 commented Nov 16, 2015

Can we also consider adding the FormatFilter globally when the format mappings are setup?

@danroth27

This comment has been minimized.

Show comment
Hide comment
@danroth27

danroth27 Nov 16, 2015

Member

It's also pretty ugly that you have to specify the full route on the first action including the api/[controller] prefix.

Member

danroth27 commented Nov 16, 2015

It's also pretty ugly that you have to specify the full route on the first action including the api/[controller] prefix.

@danroth27 danroth27 added this to the 6.0.0-rc2 milestone Nov 16, 2015

@rynowak

This comment has been minimized.

Show comment
Hide comment
@rynowak

rynowak Nov 16, 2015

Member

It's also pretty ugly that you have to specify the full route on the first action including the api/[controller] prefix.

~~You can just do [HttpGet, HttpGet(".{format}")]. Once we fix this bug [HttpGet(".{format?}")] should work.~~~

Yeah, I realize that won't work.

Member

rynowak commented Nov 16, 2015

It's also pretty ugly that you have to specify the full route on the first action including the api/[controller] prefix.

~~You can just do [HttpGet, HttpGet(".{format}")]. Once we fix this bug [HttpGet(".{format?}")] should work.~~~

Yeah, I realize that won't work.

@rynowak

This comment has been minimized.

Show comment
Hide comment
@rynowak

rynowak Nov 16, 2015

Member

Can we also consider adding the FormatFilter globally when the format mappings are setup?

Mappings are set up today by doing things like AddJsonFormatter(). We might just want to add the format filter always.

Member

rynowak commented Nov 16, 2015

Can we also consider adding the FormatFilter globally when the format mappings are setup?

Mappings are set up today by doing things like AddJsonFormatter(). We might just want to add the format filter always.

@Eilon

This comment has been minimized.

Show comment
Hide comment
@Eilon

Eilon Nov 16, 2015

Member

@rynowak thoughts on how to make the sub-attribute-route "mergeable" without the intermediate / that gets inserted? An option on HttpGet such as [HttpGet("sub-route", InsertSlash = false)] or something similar?

Member

Eilon commented Nov 16, 2015

@rynowak thoughts on how to make the sub-attribute-route "mergeable" without the intermediate / that gets inserted? An option on HttpGet such as [HttpGet("sub-route", InsertSlash = false)] or something similar?

@rynowak

This comment has been minimized.

Show comment
Hide comment
@rynowak

rynowak Nov 16, 2015

Member

No bright ideas here. [HttpGet("sub-route", InsertSlash = false)] is pretty gross. I'd rather leave [HttpGet("/api/[controller].{format?}")] than that. There are lots of places in attribute routing where we haven't optimized for being terse.

If we really want to optimize this, we could make . special as the first character of a route template.

Member

rynowak commented Nov 16, 2015

No bright ideas here. [HttpGet("sub-route", InsertSlash = false)] is pretty gross. I'd rather leave [HttpGet("/api/[controller].{format?}")] than that. There are lots of places in attribute routing where we haven't optimized for being terse.

If we really want to optimize this, we could make . special as the first character of a route template.

@Eilon

This comment has been minimized.

Show comment
Hide comment
@Eilon

Eilon Nov 16, 2015

Member

Just . would be a super obscure special case. I'm also fine doing nothing for that case now. My suggestion could be implemented at any time, if there was enough demand for it.

Member

Eilon commented Nov 16, 2015

Just . would be a super obscure special case. I'm also fine doing nothing for that case now. My suggestion could be implemented at any time, if there was enough demand for it.

@Eilon Eilon modified the milestones: Backlog, 1.0.0-rc2 Feb 26, 2016

@Eilon

This comment has been minimized.

Show comment
Hide comment
@Eilon

Eilon Feb 26, 2016

Member

Deciding not too do this for now. We don't know of a good fix and the scenario doesn't seem super common. And there's a workaround.

Member

Eilon commented Feb 26, 2016

Deciding not too do this for now. We don't know of a good fix and the scenario doesn't seem super common. And there's a workaround.

@nikibobi

This comment has been minimized.

Show comment
Hide comment
@nikibobi

nikibobi Jul 3, 2017

Hello I got the same problem I have this route attribute on one of my actions:
[Route("/{devId}/{senId}/data.{format?}")]
A workaround I found is to create a route variable with default value like this:
[Route("/{devId}/{senId}/{file=data}.{format?}")]

nikibobi commented Jul 3, 2017

Hello I got the same problem I have this route attribute on one of my actions:
[Route("/{devId}/{senId}/data.{format?}")]
A workaround I found is to create a route variable with default value like this:
[Route("/{devId}/{senId}/{file=data}.{format?}")]

@mkArtakMSFT

This comment has been minimized.

Show comment
Hide comment
@mkArtakMSFT

mkArtakMSFT Sep 11, 2018

Contributor

Closing this issue as there was no community involvement for quite a while now and there is also a workaround available.

Contributor

mkArtakMSFT commented Sep 11, 2018

Closing this issue as there was no community involvement for quite a while now and there is also a workaround available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment