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

global options are not taked into account when route options are defined partially #340

Open
2 tasks done
blephy opened this issue Jan 2, 2025 · 2 comments
Open
2 tasks done
Labels

Comments

@blephy
Copy link

blephy commented Jan 2, 2025

Prerequisites

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

Fastify version

5.2.0

Plugin version

8.0.1

Node.js version

22.12.0

Operating system

macOS

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

15.1.1

Description

Given this plugin registered has follow :

function onInvalidRequestPayload(
  ...parameters: Parameters<NonNullable<FastifyCompressOptions['onInvalidRequestPayload']>>
) {
  return {
    statusCode: HTTPStatusCodeEnum.BadRequest,
    code: 'Invalid Request Payload',
    name: 'InvalidRequestPayload',
    error: 'Invalid Request Payload',
    message: `Payload is not ${parameters[0]} encoded. ${parameters[2].message}`,
  };
}

function onUnsupportedRequestEncoding(
  ...parameters: Parameters<NonNullable<FastifyCompressOptions['onUnsupportedRequestEncoding']>>
) {
  return {
    statusCode: HTTPStatusCodeEnum.UnsupportedMediaType,
    code: 'Unsupported Media Type',
    name: 'UnsupportedMediaType',
    error: 'Unsupported Media Type',
    message: `${parameters[0]} encoding is not supported`,
  };
}
  
await fastify.register(compressPlugin, {
        global: true,
        onInvalidRequestPayload: onInvalidRequestPayload,
        removeContentLengthHeader: false,
        onUnsupportedRequestEncoding: onUnsupportedRequestEncoding,
      });

if a route is registered has follow :

fastifyInstance.withTypeProvider<FastifyZodOpenApiTypeProvider>().route({
    method: 'POST',
    url: '/',
    schema: {
      body: BODY,
      response: {
        [HTTPStatusCodeEnum.OK]: RESPONSE,
      },
    },
    decompress: {
      // eslint-disable-next-line @typescript-eslint/explicit-function-return-type, @typescript-eslint/promise-function-async
      onUnsupportedRequestEncoding: function (encoding, request, reply) {
        // we duplicate this here because our preParsing hook is not triggered in this context
        request.monitoringScanEvent =
          MonitoringScanEvent.CreateWithoutTraceId().setName('a');

        return onUnsupportedRequestEncoding(encoding, request, reply);
      },
    },
    bodyLimit: 14_680_064, // 14M
    preParsing: (request, _reply, _payload, done): void => {
      request.monitoringScanEvent =
        MonitoringScanEvent.CreateWithoutTraceId().setName('a');

      done();
    },
    errorHandler: async (error, request, reply): Promise<void> => {
      request.monitoringScanEvent.setStatus('error');
      request.monitoringScanEvent.mergeWithMessage({
        error: error.message,
      });

      fastifyInstance.errorHandler(error, request, reply);
    },
    onResponse: async (request): Promise<void> => {
      await fastifyInstance.httpClient
        .getInstance()
        .v1.monitoring.postScanEvent(
          request.monitoringScanEvent.toObject(),
          request.monitoringScanEvent.getTraceId(),
        );
    },
    handler: async (request, reply): Promise<ResponseType> => {
      const response = await handler(
        fastifyInstance,
        request.body,
        request.headers,
        request.monitoringScanEvent,
        request.log,
      );

      return await reply.status(HTTPStatusCodeEnum.OK).send(response);
    },
  });

This onInvalidRequestPayload hook provided in the global configuration at plugin registration is not taken into account, instead, the 'default' hook from the plugin is used.

Link to code that reproduces the bug

No response

Expected Behavior

This plugin should merge every options with :

  • route option first
  • global option at registration in second
  • fallback to default plugin option last
@blephy blephy changed the title global options are not well merged with route option global options are not taked into account when route option are defined partially Jan 2, 2025
@blephy blephy changed the title global options are not taked into account when route option are defined partially global options are not taked into account when route options are defined partially Jan 2, 2025
@Uzlopak
Copy link
Contributor

Uzlopak commented Jan 7, 2025

True, we just overrwrite the values.

const mergedDecompressParams = Object.assign(

const mergedCompressParams = Object.assign(

@mcollina
Copy link
Member

mcollina commented Jan 7, 2025

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@mcollina mcollina added the bug label Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants