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

Response serialization and validation #2439

Open
sher opened this issue Mar 30, 2024 · 5 comments
Open

Response serialization and validation #2439

sher opened this issue Mar 30, 2024 · 5 comments
Labels
enhancement New feature or request.

Comments

@sher
Copy link

sher commented Mar 30, 2024

What is the feature you are proposing?

Certain frameworks (eg: fastify) provide response validation for success or error payloads.
Is there support for such feature in hono? If not, is it something planned?

Context

Apart from obvious advantages like schema-first approach, this feature would greatly help securing API internals.
For example, when you have multiple unique IDs for DB records and you want to make sure that only external_id IDs are ever returned without accidentally leaking internal_id IDs.

@sher sher added the enhancement New feature or request. label Mar 30, 2024
@sher sher changed the title Response validation Response serialization and validation Mar 30, 2024
@yusukebe
Copy link
Member

Hi @sher

Thank you for sharing your opinion.

Is there support for such feature in hono? If not, is it something planned?

Hono does not have a future now. But there have been some discussions before. We tried to implement response validation in Zod OpenAPI, but declined because we thought we needed to work on Hono's core.

honojs/middleware#184

I think it would be better to make the Response Validator as thin as the current hono/validator is very thin, and let other libraries do the real validation.

@sher
Copy link
Author

sher commented Mar 30, 2024

Thanks @yusukebe

Agree, that the hono API should be as thin as possible and delegate validation to whatever library user wants to use. Currently handlers can accept multiple request validation functions. As the sequence of serialization and validation is reversed in response compared to request, it seems better to handle this inside every handler function before returning data.

const schema = z.object({
  email: z.string().email(),
  password: z.string(),
});

app.post('/me', async function (c) {
  const user = D1.findOne({email: c.body.email});
  const parsed = schema.safeParse(user);
    if (!parsed.success) {
      return c.text('Invalid!', 401);
    }
    return parsed.data;
});

But still this approach is not ideal, we want to specify the shape that has to be returned on every case and let the schema validator to handle the situation.

This makes the task bit difficult, because we have to have a common response shape declaration/specification "language", for example json-schema shape. Then the validator should take the shape and validate data against it.

@sher
Copy link
Author

sher commented Mar 30, 2024

@yusukebe

How about adding response:* keys to validators like this?

const route = app.on(
  'LIST_OF_USERS',
  '/users',
  validator('response:200', (value, c) => {
    const parsed = schema.safeParse(value);
    if (!parsed.success) {
      return c.text('Invalid!', 401);
    }
    return parsed.data;
  }),
  validator('response:400', () => {
    return {
      q: 'bar',
    };
  }),
  (c) => {
    return c.json({
      success: true,
    });
  }
);

@yusukebe
Copy link
Member

yusukebe commented Apr 3, 2024

@sher

My idea is to create another validator like responseValidator to validate response content. Below are a rough implementation. I think it's difficult to infer the response type in the handler, and the code will be complicated. But if it can run correctly, it will be very convenient.

const validationErrorResponse = (reason: string) => {
  return new Response(reason, {
    status: 401, // We have to set proper status code
  })
}
type ResponseValidatorFnResponse<T = any> =
  | {
      success: true
      data: T
    }
  | {
      success: false
      reason: string
    }

type ResponseValidatorFn<T = any> = (
  value: any
) => ResponseValidatorFnResponse<T> | Promise<ResponseValidatorFnResponse<T>>

const responseValidator =
  (status: number, validationFn: ResponseValidatorFn) => async (c, next) => {
    await next()
    if (!c.res.headers.get('content-type')?.startsWith('application/json')) {
      c.res = validationErrorResponse('The content is not JSON')
      return
    }
    if (c.res.status !== status) {
      c.res = validationErrorResponse(`The status is not ${status}`)
      return
    }
    const value = await c.res.json() // Maybe we have to clone the response body.
    const result = await validationFn(value)
    if (result.success === false) {
      c.res = validationErrorResponse(result.reason)
      return
    }
    const newResponse = new Response(JSON.stringify(result.data))
    c.res = newResponse
  }

const schema = z.object({
  success: z.boolean(),
})

app.get(
  '/results',
  responseValidator(200, (value) => {
    const parsed = schema.safeParse(value)
    if (!parsed.success) {
      return {
        success: false,
        reason: parsed.error.message,
      }
    }
    return {
      success: true,
      data: parsed.data,
    }
  }),
  (c) => {
    return c.json({
      // Is not typed currently
      success: true,
    })
  }
)

@marceloverdijk
Copy link

I have the exact same case, I don't want some fields accidentialy leaking to the client.
It would be great to have compile time validation as well.

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

No branches or pull requests

3 participants