add connInfo in custom handlers

[artpani4]

It would be really cool to be able to access connInfo from custom handlers

export const handler: Handlers = {
   async GET(_req, ctx) {
  // access to connInfo somewhere here
     return await ctx.render();
   },
};

[deer]

Everywhere connInfo appears in the source, it's typed as ServeHandlerInfo. But both HandlerContext and MiddlewareHandlerContext are declared with extends ServeHandlerInfo. So the ctx should always have the local and remote addresses available. We spread the connInfo into the contexts here and here.

I think this can be closed as already working. Am I missing something?

[git001]

let me jump in and ask what's the replacement because it looks deprecated. connInfo ==> https://github.com/denoland/fresh/blob/main/src/server/compose.ts#L64 ===> https://github.com/denoland/fresh/blob/main/src/server/types.ts#L246-L253

export type ServeHandlerInfo = {
  /**
   * Backwards compatible with std/server
   * @deprecated
   */
  localAddr?: Deno.NetAddr;
  remoteAddr: Deno.NetAddr;
};

I ask because some Applications runs behind a reverse proxy and send the client IP in http headers like X-Forwarded-For or Forwarded HTTP Extension which should be parsed and set the remoteAddr with that header value. I think this could be a good candidate for a middleware, couldn't it be?

Another interesting candidate could be the Proxy Protocol

[deer]

Hmmm, that sounds like a different issue to me. I still think this original issue is "already working".

Your request sounds like "Fresh should automatically parse X-Fowarded-For headers and use that as remoteAddr". Please correct me if I'm wrong. I guess this would be a cool feature, although it also sounds like a breaking change, if we were to start overriding the existing value with whatever is contained in this header. Instead, perhaps we could do something like this:

export type ServeHandlerInfo = {
  /**
   * Backwards compatible with std/server
   * @deprecated
   */
  localAddr?: Deno.NetAddr;
  remoteAddr: Deno.NetAddr;
  forwardedForAddr?: Deno.NetAddr;
};

But I agree, if this isn't something that gets natively supported in Fresh, it does seem like a good candidate for a plugin providing a single middleware that does this. That way people could just install the plugin, instead of having everyone reinvent the wheel.

Additionally, I think only the localAddr is being deprecated. As far as I can tell, ServeHandlerInfo as a whole is not deprecated.

Edit: I decided to try this out and I did get it working so that ServeHandlerInfo has an optional forwardedForAddr. I did this by modifying the function passed to Deno.serve. @marvinhagemeister, what do you think of this approach? If you're ok with it, I'll write some tests, document it, and open a PR.

[git001]

Hmmm, that sounds like a different issue to me. I still think this original issue is "already working".

Okay, then sorry for the hijacking. will create a new issue for that or keep the discussion here?

Edit 1:

Your request sounds like "Fresh should automatically parse X-Fowarded-For headers and use that as remoteAddr".

You are right. There are some Frameworks out there which offers this as middleware. Here a example from a golang framework.

https://github.com/go-chi/chi/blob/master/middleware/realip.go

...
func main() {
  r := chi.NewRouter()

  // A good base middleware stack
  r.Use(middleware.RequestID)
  r.Use(middleware.RealIP)
  r.Use(middleware.Logger)
  r.Use(middleware.Recoverer)
...

Edit 2: Looks like there is also something in rust axum client IP ( https://github.com/imbolc/axum-client-ip )there is also an const https://docs.rs/http/latest/http/header/constant.FORWARDED.html

[marvinhagemeister]

I feel like this is something that's easy enough to add as a middleware. I'm not sure if it's a good idea to built that into Fresh by default as it gives a false impression that the header value is trusted. The header value can be easily spoofed and is only somewhat trustworthy if you control the proxy before it and that sets the last value in the X-Forwarded-For header. I feel like Fresh itself cannot make assumptions about where it is run like that.

This should live in a custom middleware for the projects that need it, not in Fresh itself.

[git001]

@marvinhagemeister agree. Is there any middleware overview for fresh? something like "awesome rust" or "awesome golang"? I will try to create such a middleware and create a new issue for that.

[deer]

It seems like #1552 is what we need here.

[git001]

It seems like #1552 is what we need here.

Then I will create a plugin instead of middleware.

[deer]

Well, to elaborate, I was referring to your idea about how to share such a middleware. Currently middleware is specific to your particular project. But you can of course create a plugin which provides a single middleware that extracts the header.

The only question is then: how does anyone find your great plugin? We would need some sort of directory, where people can search for Fresh plugins to use. Hence the link to #1552.

Edit: https://fresh.deno.dev/docs/concepts/middleware and https://fresh.deno.dev/docs/concepts/plugins#routes-and-middlewares should provide a starting point on how to get this working. https://github.com/deer/fresh_ga4 is an example of a simple plugin providing a google analytics middleware plugin.

[git001]

how about to add a "awesome fresh" repo similar to this one for golang https://github.com/avelino/awesome-go

[git001]

I have now created https://github.com/denoland/fresh/issues/2016 and will try to start to create such a plugin.