Javalin: Validation functionality (discussion, input wanted)

[tipsy]

I'm considering including some functionality for validating user input. Typical things that need to be validated are:

ctx.pathParam("user-id")
ctx.queryParam("isbn")
ctx.formParam("username")

You could potentially include the validate function on the Context:

val isbn = ctx.validate(Param.QUERY, "isbn").matches(isbnRegex).get() ?: throw BadRequestResponse()
val username = ctx.validate(Param.FORM, "username").minLength(5).get() ?: throw BadRequestResponse()

But since there are a lot of possible entry points, a general validation function (util) would probably be better?

val isbn = validate(ctx.queryParam("isbn")).matches(isbnRegex).get() ?: throw BadRequestResponse()
val username = validate(ctx.formParam("username")).minLength(5).get() ?: throw BadRequestResponse()

This issue is intended for discussion. Should the functionality be included? If yes, how should it work?

[tipsy]

Tagging you here @grote, since you're the one who brought up validation in the Swagger issue.

[kmehrunes]

I'm totally in favor of this as long as the validation function throws the exception itself. Also we need custom validation and not just what is provided (matches() and minLength() in the example). I did something a bit similar, here is a sample code

Validation.validate(request.username, validateUsername, "Invalid username")
           .validate(request.email, validateEmail, "Invalid email");

Where validate() takes care of throwing a BadRequestResponse with the specified error message.

Another variant when only type checking is required:

double latitude = Validation.requireDouble(context.queryParam("lat"));

[tipsy]

@kmehrunes I'm more in favor of mentioning the exception explicitly. How would your sample look in a controller?

I was thinking a controller would look like this:

public static void myController(Context ctx) {

    String isbn = validate(ctx.queryParam("isbn")) // very simple to setup
        .lengthBetween(5, 10)
        .customRule(value -> {...}, "Custom message")
        .getOrThrowBadRequestResponse()

    String isbn = ctx.validate(Param.QUERY, "isbn") // a little more work to setup
        .lengthBetween(5, 10)
        .customRule(value -> {...}, "Custom message")
        .getOrThrowBadRequestResponse()

}

I don't think validation and extraction should be two separate operations.

[bchristenson]

What about something like:

public static void myController(Context ctx) {
    validate(ctx.queryParam("isbn"))
        .checkLengthBetween(5, 10)
        .getOrThrowBadRequestException();

    validate(ctx.queryParam("isbn"))
        .check(value -> {...}, message)
        .getOrThrow(message -> CustomException::new);
}

I like using the validate(ctx.queryParam("...")) rather than validate(Param.QUERY, "isbn") because it feels more consistent with normal access, however that does limit how the default exception messages are generated (ie it wouldn't be able to say something like _"The isbn query parameter must be ..."_. An option to address this could be something like:

public static void myController(Context ctx) {
    validate(ctx.queryParam("isbn"))
        .checkLengthBetween(5, 10)
        .mapMessage(message -> "The \"isbn\" query parameter "+message)
        .getOrThrowBadRequestException();
    // or
    validate(ctx.queryParam("isbn"), "The \"isbn\" query parameter")
        .checkLengthBetween(5, 10)
        .getOrThrowBadRequestException();
}

[tipsy]

however that does limit how the default exception messages are generated (ie it wouldn't be able to say something like "The isbn query parameter must be ...". An option to address this could be something like:

Another option would be to specify what kind of param it is with a paramType() method.

.getOrThrow(message -> CustomException::new);

That's a good idea. Could also do .getOrThrow(CustomException.class);

I was also wondering if it would be good to include casting in the validation

Double isbn = ctx.validate(Param.QUERY, "isbn") // a little more work to setup
        .lengthBetween(5, 10)
        .asClass(Double.class)
        .getOrThrow(CustomException.class);

[bchristenson]

I definitely like the casting idea, however you may need to provide a functional block option (ie for things like UUIDs, which don't have string constructors and instead need to call UUID::fromString).

UUID id = ctx.validate(Param.QUERY, "id")
        .cast(UUID.class, value -> UUID.fromString(value))
        .getOrThrow(CustomException.class);

[tipsy]

I definitely like the casting idea, however you may need to provide a functional block option (ie for things like UUIDs, which don't have string constructors and instead need to call UUID::fromString).

We could take care of that with a big when expression. I'd rather make our implementation a little messy than move that mess over to the end-users.

[bchristenson]

We could take care of that with a big when expression. I'd rather make our implementation a little messy than move that mess over to the end-users.

We have use cases where we cast parameters to our own custom domain types (which would be impossible for Javalin to have knowledge about). I do like the idea of having a big when expression for standard Java classes, but think that an optional override to handle classes not defined in the when expression is important.

[tipsy]

Fair enough, we'll add an overload to it.

[vladp]

does this add any runtime cost (in terms of concurrency, memory size, cost of copies of ctx) for folks, who do not need form validation?

[tipsy]

does this add any runtime cost (in terms of concurrency, memory size, cost of copies of ctx) for folks, who do not need form validation?

No, it will be completely optional and shouldn't affect performance if not used. If we go with the pure util-function it will have absolutely no affect, if we put it on Context it will just be:

fun validate(paramType: Param, key: String) = Validator(paramType, key, this)

Which should have virtually no effect if you're not using it.

[kmehrunes]

@tipsy

• I'm okay with having .getOrThrowBadRequestException(), my problem is with needing to check the validation result in an if-statement or a ternary operator, but the proposed option is good. I think your sample controller matches what I had in mind.

• I'm also leaning towards validate(value) instead of ctx.validate(), the validation part isn't really something that should a responsibility of the context. The first option is also better for validating values of a parsed JSON object.

• Perhaps we can draw some inspiration from Postman tests to see what could work better?

• For type casting we should also provide casting functions for common cases (e.g. asDouble)

• One case we need to really consider is what happens after casting. Do we consider casting a final step or can more validation be done on the cast value? For example, if I had a string value which I need first to convert to double then check the range then it should be something like this
  validate(stringval).asDouble().between(-1.0, 1.0);

[tipsy]

Thanks @kmehrunes, that's some good input.

I'm also leaning towards validate(value) instead of ctx.validate(),

The reason I'm proposing the slightly wonky ctx.validate(Param.QUERY, "isbn") syntax is because it will easily let us create very nice error messages:

@Test
fun `test getAs(clazz)`() = TestUtil.test { app, http ->
    app.get("/int") { ctx ->
        val myInt = ctx.validate(Param.QUERY, "my-qp").getAs<Int>()
        ctx.result((myInt * 2).toString())
    }
    assertThat(http.get("/int").body, `is`("Query parameter 'my-qp' cannot be null or blank"))
    assertThat(http.get("/int?my-qp=abc").body, `is`("Query parameter 'my-qp' is not a valid Integer"))
    assertThat(http.get("/int?my-qp=123").body, `is`("246"))
}

This could be fixed by asking users to write something like:

String isbn = validate(ctx.queryParam("isbn")) 
    .errorMsgPrefix("Query parameter 'isbn'")

But it will lead to pretty noisy code.

One case we need to really consider is what happens after casting. Do we consider casting a final step or can more validation be done on the cast value?

That wasn't my initial idea, but it's what I ended up doing. If we introduce asDouble() etc the class will grow very big. Things like .between(-1.0, 1.0) could be taken care of by a custom check() method?

I will push my local branch now so people can check it out and play around.

Edit: Made a PR: https://github.com/tipsy/javalin/pull/359. No review required, it's intended only for discussion.

[kmehrunes]

I'm glad you already implemented it so that we can base our discussion on something. I think I might have an idea for solving the problem of making the validation class too big, I'll experiment with it first then I'll propose it here if it worked.

[vladp]

great. thank you.

[bchristenson]

Would ctx.queryParamValidation, ctx.pathParamValidation, and ctx.formParamValidation simplify implementation and usage over ctx.validate(Param.TYPE, ...)?

[tipsy]

@bchristenson Would ctx.queryParamValidation, ctx.pathParamValidation, and ctx.formParamValidation simplify implementation and usage over ctx.validate(Param.TYPE, ...)?

That would be slightly nicer. How about putting validated as a prefix?

val age = ctx.validatedQueryParam("age").getAs<Int>()
val username = ctx.validatedFormParam("username").check({ it.length > 10 }).get()
val userId = ctx.validatedPathParam("user-id").matches("[0-9]").getAs<Long>()

Edit: If we don't want to dirty Context we could wrap these in a class:

val age = ctx.validate.queryParam("age").getAs<Int>()
val username = ctx.validate.formParam("username").check({ it.length > 10 }).get()
val userId = ctx.validate.pathParam("user-id").matches("[0-9]").getAs<Long>()

[kmehrunes]

Haven't checked your updates yet but here's what I've got so far. Let me know if you have any comments on it or if you want me to implement a viable version of it.

/* 
 * A generic container which supports validation and 
 * mapping through lambdas, useful for non-common cases.
 * The value here isn't retrieved just to highlight that it's 
 * a generic class which wraps the value.
 */
Value<String> username = Value.of("Username123")
                .validate(s -> s.length() >= 5 && s.length() <= 10)
                .map(String::toLowerCase)
                .validate(s -> s.matches("[a-z]+\\w*"));

/*
 * A string-specific validation class which provides 
 * common string validation steps. Also supports 
 * custom validation and mapping.
 * The value here is retrieved.
 */
String username = StringValue.of("Username123")
                .lengthAtLeast(5)
                .lengthAtMost(10)
                .map(String::toLowerCase)
                .matches("[a-z]+\\w*")
                .getValueOrThrow(CustomException.class, "Invalid username");

/*
 * Similar to StringValue but for dealing with double 
 * values. Also shown here casting (parsing) from 
 * string to double which might fail just like any 
 * normal validation step.
 */
Double latitude = StringValue.of("-41.324")
                .asDouble()
                .betweenInclusive(-90.0, 90.0)
                .getValueOrThrow(CustomException.class, "Invalid latitude value");

/*
 * Sometimes you don't want to throw something and 
 * handle errors in another way.
 */
Optional<Double> latitudeOpt = DoubleValue.of(-41.324).toOptional();

[tipsy]

Let me know if you have any comments on it or if you want me to implement a viable version of it.

I like @bchristenson's suggestion for putting the validator on the Context, with prefix/suffix for param type. Mainly because I'm really excited about the idea of auto-generating nice error messages.

In my last suggestion I removed getOrThrow in favor of just get, because I needed the cast to be part of the get, like getAs<Int>() (getAsOrThrow<Int>(Exception.class) doesn't work). If we used your approach with asDouble() (etc), we could go back to having getOrThrow():

val age = ctx.validatedQueryParam("age").asInt().getOrThrow()
val username = ctx.validatedFormParam("username").check({ it.length > 10 }).getOrThrow()
val userId = ctx.validatePathParam("user-id").matches("[0-9]").asLong().getOrThrow(MyException.class)

[kmehrunes]

Looks good so far. There are just a couple of concerns I'm afraid might get overlooked by that approach:

1. What can I use to validate field values of parsed JSON objects? Will the validation package expose something to help with that?
2. How will getAs<T>() act for non trivial cases? For example, if I need the final value to be of type Date. This is why I think we should also include a map function to offer the flexibility of custom object mapping.

Other than that I think everything is set.

[tipsy]

What can I use to validate field values of parsed JSON objects? Will the validation package expose something to help with that?

Since ctx.validatedQueryParam("age") creates an instance of Validator, you could also create one directly:

val validatedProp = Validator(myJsonProp)...

How will getAs() act for non trivial cases? For example, if I need the final value to be of type Date. This is why I think we should also include a map function to offer the flexibility of custom object mapping.

Currently I'm writing converters for common classes. Date would just be new Date(value):

Date::class.java -> convert(clazz) { Date(value) } as T

If an unmapped class is used, an exception is thrown:

else -> throw IllegalArgumentException("Can't auto-cast to $clazz. Use get() and do it manually.")

I don't really see any problem with allowing a custom converter though.

Edit: Added custom mapper:

val myInstant = ctx.validatedQueryParam("my-qp").getAs<Instant>{ Instant.ofEpochMilli(it.toLong()) }

[tipsy]

When I was refactoring I stumbled upon a different approach to handling custom converters. Since I'm storing converters in a map, we could let users register converters:

JavalinValidation.registerConverter(Instant.class, v -> Instant.ofEpochMilli(Long.parseLong(v)))

Then we would call

ctx.validatedQueryParam("my-qp").getAs(Instant.class)

[BloodShura]

Honestly, I think it would be better not to pollute Javalin and create another project/module for these validations. Since you use Kotlin, extension functions would fit in perfectly (for adding validation functions to Context class if needed).

Validation varies a lot for each project depending on its serialization model. For e.g., a lot of projects just use JSON (de)serialization for automatic mapping of structured data, and therefore would not benefit from any "manual" validation functionality...

[tipsy]

@BloodShura the userbase is about 50/50 Java and Kotlin devs. The current implementation is just 75 lines, and adds three namespaced functions to the Context, so I think the benefits outweigh the drawbacks. I think formParam() probably doesn't need a validated version, since people dealing with forms probably want to hand-craft their user-feedback. Then it's just 2 functions.

[grote]

For the record, I'd appreciate formParam validation or is there any other way to get POSTed values?

Also, I don't think there's a lot of overhead being introduced for validation.

[tipsy]

@grote Typically form params are sent by the browser when users submit a HTML <form></form>. If you're posting with JavaScript (or another programming language), it's common to send JSON as the request body:

axios.post('/user', {
    firstName: 'Fred',
    lastName: 'Flintstone'
})

You'd then extract this in Javalin:

val user = ctx.body<User>()

Where User would have to be a mappable class:

data class User(val firstName: String, val lastName: String)

[grote]

Thanks, but I'd rather not deserialize untrusted input. In my use-case I have few things to POST, so don't see the point of wrapping them in JSON. But as long as there's a way to apply the validation infrastructure also on ctx.formParam() I am happy.

[tipsy]

@grote I don't mind including ctx.valiatedFormParam(), but I'd like to point out that deserializing a JSON String into a POJO/data class is very different from deserializing untrusted binary-data. You actually have to make an effort to make your code exploitable. Here are two articles if you're interested:

• https://medium.com/@cowtowncoder/on-jackson-cves-dont-panic-here-is-what-you-need-to-know-54cd0d6e8062
• https://adamcaudill.com/2017/10/04/exploiting-jackson-rce-cve-2017-7525/

Edit: I've added ctx.valiatedFormParam() to the PR again.

[tipsy]

@kmehrunes I ended up adding

fun asBoolean()
fun asDouble()
fun asFloat()
fun asInt()
fun asLong()
fun <T> asClass(clazz: Class<T>)
inline fun <reified T : Any> asClass()

This is the most fun I've had working on a feature in a long time, thanks to everyone who has contributed to the discussion.

---

Validator

The Javalin Validator is a class for validating user input. It will let you do useful stuff, like:

val fromDate = ctx.validatedQueryParam("from").asClass<Instant>().getOrThrow()
val toDate = ctx.validatedQueryParam("to")
        .asClass<Instant>()
        .check({ it.isAfter(fromDate) }, "'to' has to be after 'from'")
        .getOrThrow()

Which will yield the error message:

"Query parameter 'to' with value '1262347100000' invalid - 'to' has to be after 'from'"

Feature overview

There are two classes, the Validator, which has notNullOrEmpty(), matches() and check(), as well as the methods for casting to another type.

Calling asClass<Type>() on the Validator gives you a TypedValidator<Type>, which only has the methods check() and getOrThrow().

The feature is available both standalone and on the Context.

Standalone

If called standalone, you can supply your own message prefix:

validate(jsonProp, "jsonProp").notNullOrEmpty().getOrThrow()
// -> jsonProp cannot be null or empty

If you leave out the prefix it defaults to "Value":

validate(jsonProp).notNullOrEmpty().getOrThrow()
// -> Value cannot be null or empty

Context

If called on the Context, you get nice default prefixes:

fun validatedFormParam("abc") // "Form parameter 'abc' with value '$value'" 
fun validatedPathParam("abc") // "Path parameter 'abc' with value '$value'" 
fun validatedQueryParam("abc") // "Query parameter 'abc' with value '$value'"
fun validatedBody<T>() // "Request body as $clazz" 

Custom converters

Conversion for basic data types is included on the Validator by calling:

fun asBoolean()
fun asDouble()
fun asFloat()
fun asInt()
fun asLong()

Conversion for other data types can be done by calling:

fun <T> asClass(clazz: Class<T>)
inline fun <reified T : Any> asClass()

In which case you'll have to register a converter for your class by calling JavalinValidation#register:

JavalinValidation.register(Instant::class.java) { Instant.ofEpochMilli(it.toLong()) }

[kmehrunes]

@tipsy Nice! I like the provided features. I hope other people will find them useful as well. Thanks for working on them.

[tipsy]

Out now as part of 2.2.0 !

[grote]

I should probably have thought about this earlier, but I wonder how the validator works in unit tests.

Without the validator, I can check how my controller behaves if a form parameter is missing by doing

every { ctx.formParam("text") } returns null

However, with the validator, I cannot do:

every { ctx.validatedFormParam("text") } returns Validator(null)

because it already throws an exception in its constructor.

Any advise how this could work without doing an integration test?

[tipsy]

@grote I don't feel strongly about performing that check in the constructor. You can submit a PR which moves the check to the getOrThrow() function.

[Rudge]

@tipsy Won't the getOrThrow() have Exception in its constructor? I think that is necessary, in some case, it is necessary to change or create a custom object error to response. For example in that case in realworld example.

https://github.com/Rudge/kotlin-javalin-realworld-example-app/blob/master/src/main/kotlin/io/realworld/app/web/controllers/AuthController.kt

What do you think? I need to throw one exception to get in exception mapping and set http status 422 and one custom error.

[tipsy]

@Rudge It was a bit tricky to set up, so I left it out to see if anyone would need it. If you want to submit a PR I'll review it!

[monocl-haklof]

.getOrDefault( defautValue ) would be nice for setting default value if validation fails/param not provided. In my application it is very common that query params are optional. Not sure if it would make sense to have optional formParams or pathParams though.

Edit: I think Integer page = ctx.validatedQueryParam("p","1").asInt().getOrThrow(); will do the trick. So never mind :)

[kmehrunes]

@tipsy I kinda forgot about this discussion. I worked on another version on the side and extended it to have predefined pipelines as well (the repo is here). Feel free to take whatever features you deem useful.