Directive: Optional entity and empty body

[akka-ci]

Issue by 2m
Monday Jun 15, 2015 at 07:17 GMT
Originally opened as akka/akka#17716

---

As reported in the mailing list akka-http is not able to handle a request with an empty body when route expects an optional entity.

Even an empty string is not a valid json it should still be possible to handle such a request when expected entity is optional.

[akka-ci]

Comment by jrudolph
Monday Jun 15, 2015 at 09:36 GMT

---

The issue on the mailing list was that

entity(as[Option[DeleteUserBody]])

returns a 400 response with this message:

The request content was malformed:
JSON terminates unexpectedly.

The underlying issue is what to expect for the above entity line. This is statically decided by the choice of the implicit unmarshaller.

In this case a JSON unmarshaller was chosen which depends on the body being well-formed JSON (which an empty entity isn't). A JSON unmarshaller for T is always available if a JsonFormat for T is available which is because we define a generic JsonFormat for Option[T] in spray-json. However, this works on a JsValue and not on the plain input string so it cannot be fixed on that level.

Instead, you could also lift a JSON unmarshaller for DeleteUserBody using GenericUnmarshallers.targetOptionUnmarshaller which would already do the right thing. So, the question here is how to disambiguate the two cases and make sure the right unmarshaller is chosen.

/cc @sirthias

[akka-ci]

Comment by sirthias
Monday Jun 15, 2015 at 14:51 GMT

---

That is indeed a good question. Basically we'd want to deprioritize DefaultJsonProtocol.optionFormat over GenericUnmarshallers.targetOptionUnmarshaller in this case.
However, because of the old-school way that spray-json defines its infrastructure implicits the DefaultJsonProtocol.optionFormat is usually explicitly imported, which puts it into a higher implicit priority bracket than the targetOptionUnmarshaller.

So, unfortunately, there doesn't appear to be a clean and good solution.
The best solution would probably be to remove the DefaultJsonProtocol.optionFormat in spray-json altogether (as it is quite limited anyway) and replace it with something better where is really needed (which is defining JsonFormats for case class hierarchies). However, we can't do that from akka-http.

The way I see it is that we need to address this via documentation, where we ask people to either
import spray.json.DefaultJsonProtocol.{optionFormat => _, _} in these cases or shadow optionFormat if they mix in the DefaultJsonProtocol trait.

[akka-ci]

Comment by jrudolph
Tuesday Jun 16, 2015 at 08:18 GMT

---

Would adding a local alias for the targetOptionUnmarshaller help? Maybe if you write

implicit val optionsAsEmptyUnmarshaller = Unmarshallers.targetOptionUnmarshaller[HttpRequest, DeleteUserBody]

this unmarshaller would have higher priority than any otherwise imported ones?

[akka-ci]

Comment by sirthias
Tuesday Jun 16, 2015 at 10:08 GMT

---

I haven't checked but that may provoke an implicit ambiguity if I import DefaultJsonProtocol._ instead of mixing it in as a trait.

[akka-ci]

Comment by divijan
Monday Jun 22, 2015 at 15:48 GMT

---

I was going to raise an issue that seems a sub-issue of this one. I define my own JsonSupport in the same vein as SprayJsonSupport is defined. I would like Entity Expected error message instead of

The request content was malformed:
...

when no body is provided. This also holds for non-optional entity routes. Can this be made a default behavior on the lowest level Unmarshallers, like byteStringUnmarshaller? Judging from MarshallingDirectivesTest in akka-http-tests, the user is expected to handle empty entities by hand, which, to my mind, should be handled automatically

[akka-ci]

Comment by sirthias
Monday Jun 22, 2015 at 18:24 GMT

---

@divijan If you write your JSON support yourself then you should simply reject with a RequestEntityExpectedRejection if no request entity is present.

[akka-ci]

Comment by analytically
Tuesday Oct 06, 2015 at 13:07 GMT

---

👍

[akka-ci]

Comment by ymeymann
Wednesday Mar 16, 2016 at 15:06 GMT

---

Any plans to fix this issue?

[akka-ci]

Comment by ymeymann
Wednesday Mar 16, 2016 at 15:48 GMT

---

I ended up doing

extractRequest { req =>
  val action = (opt: Option[MyEntityType]) => ???
  if (req.entity.contentLengthOption.contains(0L)) {
    action(None)
  } else {
    entity(as[MyEntityType]) { e =>
      action(Some(e))
    }
  }
}

Is it a reasonable workaround?

[akka-ci]

Comment by bandrzejczak
Thursday Aug 11, 2016 at 14:42 GMT

---

@ymeymann You can enclose it into your own directive:

def optionalEntity[T](unmarshaller: FromRequestUnmarshaller[T]): Directive1[Option[T]] =
  extractRequest.flatMap { req =>
    if (req.entity.contentLengthOption.contains(0L)) {
      provide(Option.empty[T])
    } else {
      entity(unmarshaller).flatMap(e => provide(Some(e)))
    }
  }

[akka-ci]

Comment by analytically
Thursday Aug 11, 2016 at 14:43 GMT

---

This should be part of Akka HTTP.

[akka-ci]

Comment by bandrzejczak
Thursday Aug 11, 2016 at 16:02 GMT

---

Actually I've just checked and it doesn't work for empty body. What I've actually used and tested is:

def optionalEntity[T](unmarshaller: FromRequestUnmarshaller[T]): Directive1[Option[T]] =
  entity(as[String]).flatMap { stringEntity =>
    if(stringEntity == null || stringEntity.isEmpty) {
      provide(Option.empty[T])
    } else {
      entity(unmarshaller).flatMap(e => provide(Some(e)))
    }
  }
}

Seems hackish, but I haven't found better way to check whether the body is empty. entity.isKnownEmpty() doesn't work well for this case.

[akka-ci]

Comment by genarorg
Monday Aug 29, 2016 at 21:14 GMT

---

how about just providing an alternate route if you get a rejection from the entity directive:

entity(as[SomeModel]) { model => 
  complete(OK, "a request body was received") 
}
~  complete(OK, "no request body received")

[samudurand]

I tried the solution from @bandrzejczak and it works perfectly for me, I think this should be part of akka http

[gregdingle]

I also used the solution from @bandrzejczak after much searching around.

[morsecodist]

I do not recommend the solution from @bandrzejczak. When receiving traffic from slower connections it can read an incomplete post body and then attempt to parse it. This can result in intermittent JSON parsing errors.

[decoursin]

@morsecodist Are you sure? Why would it not wait for the entire string to be ready?