Issues with http header configuration

[404pilot]

Hi, I am trying to use the http header to specify the version.

version 11 and version 1 are the same

MyApi::Application.routes.draw do
  api_version(:module => "V1", :header => {:name => "Accept", :value => "application/vnd.mycompany.com; version=1"}) do
    match '/foos.(:format)' => 'foos#index', :via => :get
    match '/foos_no_format' => 'foos#index', :via => :get
    resources :bars
  end
end

I am following the example and find out if I have a header named "application/vnd.mycompany.com; version=11", the application will still return 200 instead of 404 since I specify a higher version.

MyApi::Application.routes.draw do
  api_version(:module => "V20120317", :header => {:name => "API-VERSION", :value => "1"}, :default => true) do
    match '/foos.(:format)' => 'foos#index', :via => :get
    match '/foos_no_format' => 'foos#index', :via => :get
    resources :bars
  end
end

With this example, I specifiy "1" as the defaulth version. If I have a invalid header like "API-VERSION: 9" which is not supported yet, but the application is still going to return 200 which I guess it will use the default version. Should this return 404 since we specify an unsupported version?

[bploetz]

Sorry, I'm not sure I understand what problem you're having. Can you send me your entire routes file, the requests you're sending and their associated responses, and highlight which responses are wrong?

Thanks.

[oliver-hohn]

Hi all,

Based on @404pilot's comment, the issue seems to be when having multiple versioning strategies and the default flag set. I am having the same problem, @bploetz could you re-open this issue?

Setup to replicate

If you start off with a fresh Rails project, with a V1::FoosController, that will render bar:

class V1::FoosController < ApplicationController
  def index
    render plain: 'bar'
  end
end

And the following routes.rb:

Rails.application.routes.draw do
  # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html

  api_version(module: 'V1', header: { name: 'Accept', value: 'application/vnd.mycompany.com; version=1'}, path: { value: 'v1'}, default: true) do
    match '/foo' => 'foos#index', via: :get
  end
end

Running rake routes will produce:

Prefix Verb URI Pattern       Controller#Action
   foo GET  /foo(.:format)    v1/foos#index
v1_foo GET  /v1/foo(.:format) v1/foos#index
default_foo GET  /foo(.:format)    v1/foos#index

Issue

The default flag should indicate that if no versioning strategy can be used, i.e. the path version and the header are missing in the request, then the routes in the default block should be used to attempt to map the request to them.

For example, in the following request the path version and the header are missing, but the request can still be mapped to the FoosController#index:

curl -I localhost:3000/foo

HTTP/1.1 200 OK
...

If the request cannot be mapped to any of the routes in the default block, then a 404 is raised (which is expected):

curl -I localhost:3000/invalid_route

HTTP/1.1 404 Not Found
...

When a versioning strategy is used in the request, then it should be enforced that the request can only be mapped to routes declared under that versioning strategy. For example, if the path versioning strategy is used in the request, with value v2, and there are no routes under the v2 path versioning strategy, then a 404 should be returned. Which in this case does happen:

curl -I localhost:3000/v2/foo

HTTP/1.1 404 Not Found
...

The same should apply to the header versioning strategy. When specifying a header value, for which there are no routes, then a 404 should be raised. However, a 200 is returned:

curl -I localhost:3000/foo -H "Accept: application/vnd.mycompany.com; version=invalid"

HTTP/1.1 200 OK
...

This seems to be because it is falling back to the default path versioning, which would map /foo to /v2/foo, thus ignoring the header versioning strategy.

Note this was done in Rails 5.1 using versionist 1.7.0.

Remediation

To my mind, the strategy for figuring out where to map the request to should be as follows:

1. Are there any header versioning strategies?
  1.1 If so, check if the `Accept` header (or custom header) is present.
      1.1.1 If the `Accept` header (or custom header) is not present, then move to 2.
      1.1.2 If the `Accept` header (or custom header) is present, check it's value, and check if there are any routes under that value.
        1.1.2.1 If there are no routes under that value, then raise a 404 (end)
        1.1.2.2 If there is a route under that value, then pass on the request to that Controller (end)
  1.2 If there are no header versioning strategies, skip to 2.
2. Is there a path versioning strategy present?
  2.1 If so, check the path
    2.1.1 If the path does not seem to follow the version syntax, skip to 3.
    2.1.2 If the path follows the version syntax, then check it's value, and check if there are any routes under that value.
      2.1.2.1 If there are no routes under that value, then raise a 404 (end)
      2.1.2.2 If there is a route under that value, then pass on the request to that Controller (end)
  2.2. If there are no path versioning strategies present, then skip to 3.
3. Is there a request parameter strategy present?
  3.1 If so, check if the request parameter is present
    3.1.1 If it is not present, then skip to 4.
    3.1.2 If it is present, check it's value, and if there are any routes under that value.
      3.1.2.1 If there are no routes under that value, then raise a 404 (end)
      3.1.2.2 If there is a route under that value, then pass on the request to that Controller (end)
  3.2 Skip to 4.
4. Let Rails routing handle the reset. If it can't find a route then it will raise a 404. (end)

Where "(end)" marks either a Controller handling the request, or a 404 being raised.

[bploetz]

@oliver-hohn but here's the problem: versionist (currently) has no way of knowing that a value in a header (which, in the case of the Accept header, there can be many), represents a version string. In other words, if the Accept header contains:

Accept: application/json,
              text/plain; q=0.5, text/html,
              application/vnd.mycompany.com-someunknownversion,
              text/x-dvi; q=0.8, text/x-c

It doesn't know that that application/vnd.mycompany.com-someunknownversion nestled in there represents a version string. It simply looks to see any of the configured versioning strategies match the incoming request, and if not, serves up the default value, if configured.

For what you describe to work, we'd need to know the "template" of what a version string looks like (application/vnd.mycompany.com-XXX) and if we see strings of that format in the header, treat them differently (i.e. serve up a 404 if we get a version string which matches the template but doesn't match any configured versions, rather than serving up the default version as it does today).

Also, your proposed logic in the Remediation section does not specify how default handling should work.

[oliver-hohn]

@bploetz Thanks for the reply. I can see the complexity in determining what a version string would look like, and that the proposed logic is not fully fledged out.

Would it be fair to say then, that using the Header strategy along with the default flag set to true, would mean that invalid Header version values would be ignored, and the default routes would be used?

[bploetz]

@oliver-hohn based on your assessment above, that appears to be the case.

[bploetz]

I've re-opened this issue now that it's clearer what the issue is. I'll take a look and see what can be done about this.....