CORS Preflight doesn't works about hyper-express HOT 15 CLOSED

Hi @neo773 could you try creating the catchall route with a "/" instead of just "". Just want to see If this problem can be targeted to the middleware caching based on the route pattern.

from hyper-express.

Hi @neo773 could you try creating the catchall route with a "/" instead of just "". Just want to see If this problem can be targeted to the middleware caching based on the route pattern.

Just tried it doesn't work

from hyper-express.

Could you provide the route values after startup by logging the Server.routes property. If you have other sensitive routes, then feel free to only provide the values for the specific routes by traversing the object in the following manner: Server.routes['options']['/api/login']. Be sure that the middlewares property is visible for each Route when logging to console. This could give more clarification as to whether the internal route handling logic parsed something incorrectly or if this is a problem from uWebsockets.js

from hyper-express.

  any: {},
  get: {
    '/api/profile': Route {
      app: Server { _options: {}, locals: {} },
      method: 'GET',
      pattern: '/api/profile',
      handler: [AsyncFunction: profile],
      options: {
        middlewares: [
          { priority: 0, middleware: [AsyncFunction (anonymous)] },
          { priority: 0, middleware: [AsyncFunction (anonymous)] },
          { priority: 0, middleware: [Function (anonymous)] },
          {
            priority: 2,
            middleware: [AsyncFunction: useAuthentication]
          }
        ]
      },
      streaming: {},
      max_body_length: undefined,
      path_parameters_key: []
    },

  },
  post: {
    '/api/login': Route {
      app: Server { _options: {}, locals: {} },
      method: 'POST',
      pattern: '/api/login',
      handler: [AsyncFunction: login],
      options: {
        middlewares: [
          { priority: 0, middleware: [AsyncFunction (anonymous)] },
          { priority: 0, middleware: [AsyncFunction (anonymous)] },
          { priority: 0, middleware: [Function (anonymous)] },
          { priority: 2, middleware: [AsyncFunction: useCaptcha] }
        ]
      },
      streaming: {},
      max_body_length: undefined,
      path_parameters_key: []
    },
 
    '/api/checkout': Route {
      app: Server { _options: {}, locals: {} },
      method: 'POST',
      pattern: '/api/checkout',
      handler: [AsyncFunction: checkout],
      options: {
        middlewares: [
          { priority: 0, middleware: [AsyncFunction (anonymous)] },
          { priority: 0, middleware: [AsyncFunction (anonymous)] },
          { priority: 0, middleware: [Function (anonymous)] },
          {
            priority: 2,
            middleware: [AsyncFunction: useAuthentication]
          }
        ]
      },
      streaming: {},
      max_body_length: undefined,
      path_parameters_key: []
    },
    }
  },
  del: {},
  head: {},
  options: {},
  patch: {},
  put: {},
  trace: {},
  upgrade: {},
  ws: {}
}

from hyper-express.

The object posted above shows that internally your /api/login route is registered as a POST route and that you have no options routes registered at all. Something doesn't seem to be adding up, could provide short application code where you create the routes and can recreate the bug everytime?

from hyper-express.

That's kinda my point say you have 50 API routes you'd need to make 50 OPTIONS routes for those routes.

The way CORS preflight works is it sends the OPTIONS preflight request to the active route you're trying to access

So POST /api/profile would result in OPTIONS /api/profile

from hyper-express.

No, I understand that. What I'm saying is that even the * catchall route for OPTIONS does not show in the options object of the routes object you provided above. Can you try creating an OPTIONS route with a specific pattern like /api/login instead of * and then seeing if that leads to the route being created in the routes object?

from hyper-express.

Hi,

Sorry was a little confused and the options route was commented out when I posted that log.
Attached new log & reproducible demo

{
  any: {},
  get: {
    '/api/profile': Route {
      app: Server { _options: {}, locals: {} },
      method: 'GET',
      pattern: '/api/profile',
      handler: [Function (anonymous)],
      options: { middlewares: [], streaming: {} },
      streaming: {},
      max_body_length: undefined,
      path_parameters_key: []
    }
  },
  post: {},
  del: {},
  head: {},
  options: {
    '*': Route {
      app: Server { _options: {}, locals: {} },
      method: 'OPTIONS',
      pattern: '*',
      handler: [Function (anonymous)],
      options: { middlewares: [], streaming: {} },
      streaming: {},
      max_body_length: undefined,
      path_parameters_key: []
    },
    '/': Route {
      app: Server { _options: {}, locals: {} },
      method: 'OPTIONS',
      pattern: '/',
      handler: [Function (anonymous)],
      options: { middlewares: [], streaming: {} },
      streaming: {},
      max_body_length: undefined,
      path_parameters_key: []
    }
  },
  patch: {},
  put: {},
  trace: {},
  upgrade: {},
  ws: {}
}

Reproducible demo

const HyperExpress = require('hyper-express')

const webserver = new HyperExpress.Server()
const router = new HyperExpress.Router()

router.get('/profile', (request, response)=> {
    return response.send('hello')
})

webserver.use('/api', router)

webserver.options('*', (request, response)=> {
    response.header('x-test', 'x-test-value')
    return response.send('')
})

webserver.options('/', (request, response)=> {
    response.header('x-test', 'x-test-value')
    return response.send('')
})

webserver.listen(4000)

from hyper-express.

Hey, sorry been busy this past week. So it seems like the route is being created fine under the hood but the OPTIONS requests are not treated as a wildcard. I'll try and do more research into seeing If this is a uWebsockets.js bug or a bug with HyperExpress because If this was a problem in how HyperExpress handled requests then I would assume other method type routes would also be broken which isn't the case here.

from hyper-express.

You just need to use /* here is a working example:

import HyperExpress from 'hyper-express';

const webserver = new HyperExpress.Server();
const router = new HyperExpress.Router();

webserver.use('/', router);

router.get('/profiles', (request, response) => {
    response.setHeader('Content-Type', 'text/html; charset=utf-8');
    response.end('<html><body>Profiles</body></html>');
});

router.get('/profiles/:id', (request, response) => {
    response.setHeader('Content-Type', 'text/html; charset=utf-8');
    response.end(`<html><body>Profile #${request.params.id}</body></html>`);
});

router.options('/*', (request, response) => {
    response.header('x-test', 'x-test-value');
    return response.send('');
});

router.get('/', (request, response) => {
    response.setHeader('Content-Type', 'text/html; charset=utf-8');
    response.end(`
        <!DOCTYPE html>
        <html>
            <head>
                <title>HyperExpress CORS</title>
            </head>

            <body>
                <h1>HyperExpress CORS</h1>
                <p>
                    a simple webserver with CORS support
                </p>

                <br />

                <p>
                    <a id="check-profiles" href="#no-js">Check Profiles</a>
                </p>

                <p>
                    <a id="check-profile-1" href="#no-js">Check Profile 1</a>
                </p>

                <p>
                    <a id="check-profile-2" href="#no-js">Check Profile 2</a>
                </p>

                <script>
                    const checkProfiles = document.querySelector('#check-profiles');
                    const checkProfile1 = document.querySelector('#check-profile-1');
                    const checkProfile2 = document.querySelector('#check-profile-2');

                    function sendTestRequest(url) {
                        fetch(url, {
                            method: 'OPTIONS',
                        }).then(response => {
                            alert(response.status + ': ' + response.statusText + '\\n' + response.headers.get('x-test'));
                        }).catch(error => {
                            alert(error);
                        });
                    }


                    checkProfiles.addEventListener('click', (event) => {
                        event.preventDefault();
                        sendTestRequest('/profiles');
                    });

                    checkProfile1.addEventListener('click', (event) => {
                        event.preventDefault();
                        sendTestRequest('/profile/1');
                    });

                    checkProfile2.addEventListener('click', (event) => {
                        event.preventDefault();
                        sendTestRequest('/profile/2');
                    });
                </script>
            </body>
        </html>
    `);
});

webserver
    .listen('127.0.0.1', 3000)
    .then(() => {
        console.log('Webserver started on port 3000');
    })
    .catch((error) => {
        console.error('Could not start webserver');
        console.error(error);
        process.exit(1);
    });

from hyper-express.

Hey, sorry been busy this past week. So it seems like the route is being created fine under the hood but the OPTIONS requests are not treated as a wildcard. I'll try and do more research into seeing If this is a uWebsockets.js bug or a bug with HyperExpress because If this was a problem in how HyperExpress handled requests then I would assume other method type routes would also be broken which isn't the case here.

uWebsockets.js only supports /* not * for OPTIONS. I don't know if it's a bug tho because uWebsockets.js only documents /* too.

from hyper-express.

@neo773 could you test @UnlimitedBytes recommendation for the pattern? If that ends up working then I'd assume the solution would be to internally translate the * to /* for all routes under the hood.

from hyper-express.

@kartikk221 It worked. I have a published the middleware to NPM
Btw could you also review my PR to hyper-express-body-parser? I added TypeScript support

from hyper-express.

@kartikk221 It worked. I have a published the middleware to NPM

Btw could you also review my PR to hyper-express-body-parser? I added TypeScript support

Glad it all worked out. I briefly saw your PR already and plan on merging it once I am ready to move forward with the development of the middleware. Been busy with work past few weeks, so will likely get to it sometime next week. Thanks!

from hyper-express.

@kartikk221, any updates on PR?

from hyper-express.