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

preHandler called twice when rejecting CORS #290

Closed
2 tasks done
10xLaCroixDrinker opened this issue Jan 19, 2024 · 5 comments
Closed
2 tasks done

preHandler called twice when rejecting CORS #290

10xLaCroixDrinker opened this issue Jan 19, 2024 · 5 comments

Comments

@10xLaCroixDrinker
Copy link

10xLaCroixDrinker commented Jan 19, 2024

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.25.2

Plugin version

8.5.0

Node.js version

20.10.0

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

13.6.1

Description

When the delegator returns a false origin, the preHandler hook is called twice for the request

Steps to Reproduce

I've created a sample application that reproduces the issue:

https://github.com/10xLaCroixDrinker/fastify-cors-issue-290

There are instructions in the README.

Expected Behavior

preHandler hooks should only be called once for a single request.

@mcollina
Copy link
Member

That is expected because that preHandler is applied to your not found handler as well.

This should solve it:

'use strict'

const Fastify = require('fastify')
const fastifyCors = require('@fastify/cors');
const hookLoggingPlugin = require('./hookLogging');

(async function startServer () {
  const fastify = Fastify({
    logger: {
      transport: {
        target: 'pino-pretty',
      },
    },
  });
  fastify.register(hookLoggingPlugin);

  fastify.register(async function (fastify) {

  fastify.register(fastifyCors, {
    // hook: 'preHandler',
    delegator: (req, callback) => {
      const corsOptions = { origin: false };
      if (/^success$/m.test(req.headers.origin)) {
        corsOptions.origin = true;
      }
      callback(null, corsOptions);
    },
  });

  fastify.get('/', async function (request, reply) {
    return reply.code(200).type('text/plain').send('Hello world');
  })

  fastify.post('/', async function (request, reply) {
    return reply.code(200).type('text/plain').send('Hello world');
  })

  fastify.setNotFoundHandler(async (_request, reply) => {
    reply.code(404).type('text/plain').send('Not found');
  });

  })
  
  return fastify.listen({ port: 3000 });
})();

@10xLaCroixDrinker
Copy link
Author

That did not resolve it

@10xLaCroixDrinker
Copy link
Author

Interestingly, this does not happen when using fastify-cli

@mcollina
Copy link
Member

Oh, I've

This should solve it:

```js
'use strict'

const Fastify = require('fastify')
const fastifyCors = require('@fastify/cors');
const hookLoggingPlugin = require('./hookLogging');

(async function startServer () {
  const fastify = Fastify({
    logger: {
      transport: {
        target: 'pino-pretty',
      },
    },
  });

  fastify.register(async function (fastify) {
  // the logging should be inside ;)
  fastify.register(hookLoggingPlugin);

  fastify.register(fastifyCors, {
    // hook: 'preHandler',
    delegator: (req, callback) => {
      const corsOptions = { origin: false };
      if (/^success$/m.test(req.headers.origin)) {
        corsOptions.origin = true;
      }
      callback(null, corsOptions);
    },
  });

  fastify.get('/', async function (request, reply) {
    return reply.code(200).type('text/plain').send('Hello world');
  })

  fastify.post('/', async function (request, reply) {
    return reply.code(200).type('text/plain').send('Hello world');
  })

  fastify.setNotFoundHandler(async (_request, reply) => {
    reply.code(404).type('text/plain').send('Not found');
  });

  })
  
  return fastify.listen({ port: 3000 });
})();

@10xLaCroixDrinker
Copy link
Author

Thank you Matteo!!

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

No branches or pull requests

2 participants