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

dot in route param prevents route match #4216

Closed
kailniris opened this issue Oct 17, 2017 · 6 comments
Closed

dot in route param prevents route match #4216

kailniris opened this issue Oct 17, 2017 · 6 comments

Comments

@kailniris
Copy link

Sails version: v1.0.0-37
Node version: v8.5.0
NPM version: 5.0.3
DB adapter name: N/A
DB adapter version: N/A
Operating system: windows 10 pro


Hi,

I have a controller which serves an image

I am trying to get this route to work

items/images/myimg.jpg

It responses 404 but work with

items/images/myimg

Also works with

items/images/myimg.jpg/

I tried the following router configuration

'get /items/images/:imageName': {
  action: 'items/images/find',
  skipAssets: true,
}

If I set skipAssets: false then the response will be unauthorized. I have the following ACL

'*': false,
'items/images/find': 'isLoggedIn',

In the isLoggedIn.js policy the req.session is undefined even when the user has a valid session.

if I set 'items/images/find': true it will work but I want access control for this route.

@sailsbot
Copy link

@kailniris Thanks for posting, we'll take a look as soon as possible.


For help with questions about Sails, click here. If you’re interested in hiring @sailsbot and her minions in Austin, click here.

@Josebaseba
Copy link
Contributor

Add this method in your config/session.js file:

// We want the session in all the routes, even the "files"
  isSessionDisabled: function (req){
    return false;
  },

@kailniris
Copy link
Author

Doing what you said and setting skipAssets to false for this route solved the issue.

But why does it work? Can you point me to the documentation of this feature pls?

@sgress454
Copy link
Member

Thanks @Josebaseba!

@kailniris docs for isSessionDisabled are on the Sails 1.0 session config reference page. In a nutshell, this method (which replaces the earlier sails.config.routesDisabled setting) allows you to exempt certain routes from using the session. In most cases, loading the session for each asset is a waste of resources (and can even lead to hard-to-debug race conditions when implementing a login system via AJAX). Your case is an exception. Returning a blanket false in the isSessionDisabled function will definitely work, but if you only need session support for one route, you might consider something more targeted, e.g.:

isSessionDisabled: function (req){
  // Allow session for all item image requests.
  if (req.path.match(/^\/items\/images\//) { 
    return false; 
  }
  // Otherwise, disable session for all requests that look like assets.
  return !!req.path.match(req._sails.LOOKS_LIKE_ASSET_RX);
}

@kailniris
Copy link
Author

@sgress454
Thanks for the explanation!

However there is one thing I still don't understand,
I have this policy:
'*': false

And i use this session option:

isSessionDisabled() {
  return false;
}

If I understand it correctly sails should not serve static assets this way, but it still serves the index.html from the assets folder.

@sgress454
Copy link
Member

@kailniris Policy mappings declared in config/policies.js (including *) apply solely to controller actions. If a route isn't handled by a file in the api/controllers folder, then the only way to apply a policy to it is by using the policy route target syntax in config/routes.js.

You typically wouldn't want any policies to apply to the /index.html route, but if you did, you'd do something like the following at the top of your config/routes.js:

'GET /index.html': { policy: 'somePolicyAppliedToIndexFile' }

This would apply a policy to the route, but not actually declare a handler for it, since the www (aka "static") middleware is responsible for serving the default index.html file.

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

No branches or pull requests

4 participants