Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jetty-users] BadMessageException

On 22/05/2023 18:55, Joakim Erdfelt wrote:
Don't blindly call request.getParameter() without fully (and I do mean FULLY) understanding the side effects.

Now you tell me... :) :)

A call to request.getParameter() can read the request body content
as well. (per Servlet spec). A call to request.getParameter() can
fail for bad input, bad encoding, bad expectations, IO exceptions,
etc (it's quite a long list of causes). Since request.getParameter()
has no checked exceptions, you are stuck with RuntimeException.

But presumably all of the above are things outside of my control -- so
shouldn't I just report any exception from a call to getParameter() as a 400?

I suppose I'm going to have to browse the Jetty source code in order to be able to use it properly... :(

Since request.getParameter() has no checked exceptions, you are
stuck with RuntimeException.

True, but there are RuntimeExceptions and RuntimeExceptions. Perhaps
deriving BME from something like MalformedParametersException would have been a better idea, but too late for that I suppose.

I wouldn't catch all exceptions in my ErrorPageErrorHandler, but
only specific ones that I care about presenting to users.

I catch the exceptions I know and care about, and turn them into
meaningful messages for the end users. It's the unexpected ones I want
to be told about, since they usually indicate a programming error (e.g.
NPE, IndexOutOfBounds...) so in those cases I give an "oops, we goofed"
error message to the user, and send an email to the developers.

In this case it isn't an "oops, we goofed" situation. So it sounds like
my Plan B might be the best thing to do:

Plan B: Call request.getParameter() inside a try block at the start of request processing, and return a 400 response (as this exception seems to be trying to do) if an exception is thrown.

So I have put this (with "kludge marks"around it) before I start looking at the request, and it seems to work:

  try {
    request.getParameter("foo"); // doesn't matter what
  }
  catch (RuntimeException e) {
    if (e.getCause() instanceof IOException) {
      throw e;    // I deal with wrapped IOExceptions elsewhere
    }
    response.sendError(400,"Bad Request");
    return;
  }

And now I wonder, how many other things are lurking there ready to bite me? I always assumed that, like a request to "/foo%", the problem would be detected when the request was processed, before my code ever got to see it, and then passed straight to the error handler defined in web.xml.

Oh well, it all helps to keep me in work I suppose. :)

Thanks for the response,
--
John English



Back to the top