ring-clojure / ring

Clojure HTTP server abstraction
MIT License
3.76k stars 520 forks source link

Option to disable stacktrace printing when using jetty adapter #353

Closed raffomania closed 5 years ago

raffomania commented 5 years ago

I'm running my application using the jetty adapter, as described in the docs. However, I can't find any option to disable stacktrace printing in responses! This is unfortunate as jetty is now showing my db password to everybody when I have an error in my jdbc dbspec.

weavejester commented 5 years ago

You can just catch the exception thrown by the handler, then return a static error response:

(defn wrap-hide-exceptions [handler error-response]
  (fn [request]
    (try (handler request)
         (catch Exception _ error-response))))
raffomania commented 5 years ago

Cool, I'd suggest two things -

  1. Would this be nice to have in ring-defaults?
  2. I see that lein-ring has the :stacktraces? option to do this, would it make sense to set this to false when LEIN_NO_DEV is set?

I'm suggesting this because I was very surprised when seeing this in my live app, and think it's very reasonable to disable this as soon as some indication of a production environment is present.

raffomania commented 5 years ago

Also, with your proposed solution, I would have to re-implement error logging - not optimal...

weavejester commented 5 years ago

Also, with your proposed solution, I would have to re-implement error logging - not optimal...

You can combine it with ring.middleware.stacktrace/wrap-stacktrace-log.

I might create a separate "ring-errors" or "ring-production" library to collect useful middleware functions like this, which in turn can be included in ring-defaults. However, this not currently a high priority for me.

Another option is to use a more complete web framework, which may have functions like this built in. Duct, for example, handles this for you. Ring is a lower level abstraction layer, which means you have typically have to do a little more leg work.

raffomania commented 5 years ago

You can combine it with ring.middleware.stacktrace/wrap-stacktrace-log.

Thanks, that's a good tip.

Ring is a lower level abstraction layer

I think it's important to distinguish between "added security features" and "included insecure features". I would argue that it makes sense for a lower level abstraction layer to remove a feature that is clearly problematic in lots of use-cases, making the software simpler. I'm actually kind of surprised - did no one else bring this up yet?

Also, is this behavior a part of ring or a part of jetty? I didn't find any reference of it in the docs.

weavejester commented 5 years ago

I would argue that it makes sense for a lower level abstraction layer to remove a feature that is clearly problematic in lots of use-cases, making the software simpler.

What you're seeing probably isn't a feature in Ring, unless you've added wrap-stacktrace to your middleware stack. You're likely seeing Jetty's default response to an unhandled exception.

raffomania commented 5 years ago

Alright, I should probably read into jetty deployment. Thanks for the patience and quick explanations!