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

.NET 8 ASP Versioning is allowing any version number to be passed in to api request and returns data. #1093

Closed
1 task done
Cbutterfi opened this issue Jun 7, 2024 · 5 comments

Comments

@Cbutterfi
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

I only want routing for v1. any other integer version should 404.
Currently I can pass through any integer and get back data as if it was calling v1.
ex) GET https://somedomain/app/api/v25/resouce/641B3CAA-1229-4AC2-BA9A-73534B7E9D4C
returns the same as https://somedomain/app/api/v1/resouce/641B3CAA-1229-4AC2-BA9A-73534B7E9D4C

Expected Behavior

only v1 should hit my v1 controller. v25, v2, v8, etc should all 404.

Steps To Reproduce

using fairly boiler plate setup

in startup:
private void ConfigureApiVersioning(IServiceCollection services)
{
services
.AddApiVersioning(options =>
{
options.ReportApiVersions = true;
})
.AddApiExplorer(options =>
{
options.DefaultApiVersion = new ApiVersion(1, 0);
const string groupNameFormat = "'v'VVV";
options.GroupNameFormat = groupNameFormat;
options.SubstituteApiVersionInUrl = true;
});
}
In my controller:
[ApiVersion("1.0")]
[Route("v{api-version:apiVersion}/someResource")]
[Produces("application/json")]
[Consumes("application/json")]
public class SomeController

Exceptions (if any)

No response

.NET Version

8.0.300

Anything else?

No response

@commonsensesoftware
Copy link
Collaborator

commonsensesoftware commented Jun 7, 2024

It appears you are migrating from an earlier version and you've missed adding .AddMvc(). This is required if you use controllers; otherwise, only Minimal APIs are enabled.

private void ConfigureApiVersioning(IServiceCollection services)
{
    services
        .AddApiVersioning(options => options.ReportApiVersions = true)
        .AddMvc() // required in 6.0+ for MVC Core with controllers
        .AddApiExplorer(options =>
         {
            options.DefaultApiVersion = new ApiVersion(1, 0);
            options.GroupNameFormat = "'v'VVV";
            options.SubstituteApiVersionInUrl = true;
         });
}

@Cbutterfi
Copy link
Author

hey @commonsensesoftware, thanks for the reply.
You are correct that we are migrating from .NET6. We do already have that line for .AddMvc()

I just spun up a fresh api and seem to be able to dupe this issue.

here is my program.cs
`using Asp.Versioning;

var builder = WebApplication.CreateBuilder(args);

// Add services to the container.

builder.Services.AddControllers();
// Learn more about configuring Swagger/OpenAPI at https://aka.ms/aspnetcore/swashbuckle
builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen();
builder.Services.AddMvc(options =>
{
options.EnableEndpointRouting = false;
});
ConfigureApiVersioning(builder.Services);
var app = builder.Build();

// Configure the HTTP request pipeline.
if (app.Environment.IsDevelopment())
{
app.UseSwagger();
app.UseSwaggerUI();
}

app.UseHttpsRedirection();

app.UseAuthorization();
app.UseMvc(options =>
{

});

app.MapControllers();

app.Run();

void ConfigureApiVersioning(IServiceCollection services) => services
.AddApiVersioning(options =>
{
options.ReportApiVersions = true;
options.ApiVersionReader = new UrlSegmentApiVersionReader();
})
.AddApiExplorer(options =>
{
options.DefaultApiVersion = new ApiVersion(1, 0);
const string groupNameFormat = "'v'VVV";
options.GroupNameFormat = groupNameFormat;
options.SubstituteApiVersionInUrl = true;
});`

and here is the small change to the demo controller:
`using Asp.Versioning;
using Microsoft.AspNetCore.Mvc;

namespace NET8VersioningTest.Controllers
{
[ApiVersion("1.0")]
[Route("v{api-version:apiVersion}/[controller]")]
[ApiController]
public class WeatherForecastController : ControllerBase`

Probably something dumb I am doing, but any suggestion would be greatly appreciated!
thanks

@Cbutterfi
Copy link
Author

this works actually.

Setting options.EnableEndpointRouting = True and removing the lines: app.UseMvc(options =>
{

});

fixed it.

Thanks for your time.

@commonsensesoftware
Copy link
Collaborator

I definitely would not recommend using the legacy IRouter implementation. There are problematic edge cases. Endpoint Routing has been the way for many years now.

I think there may be some confusion as to what is happening.

builder.Services.AddMvcCore(); // or .AddMvc()

is NOT the same as:

builder.Services.AddApiVersioning().AddMvc();

The first one brings in MVC Core (or full MVC). The second one adds MVC (Core) support for API Versioning.

@Cbutterfi
Copy link
Author

Versioning was working for me in the sample WeatherForecast api I spun up, but wouldn't work in our actual project. I noticed that our Controller was missing the [APIController] Attribute. Once I added that attribute, versioning started to work great.
Adding that attribute seems to round all of our existing 400-level responses down to a 400. Something else to chase down!

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

No branches or pull requests

2 participants