akka-http icon indicating copy to clipboard operation
akka-http copied to clipboard

Timeout-Access header always contains incorrect value "<function1>"

Open akka-ci opened this issue 9 years ago • 9 comments

Issue by nartamonov Tuesday Mar 22, 2016 at 16:00 GMT Originally opened as https://github.com/akka/akka/issues/20120


Following path returns response "Timeout-Access = ":

  path("test") {
    get {
      optionalHeaderValueByName("Timeout-Access") { timeout =>
        complete(s"Timeout-Access = ${timeout.get}")
      }
    }
  }

It's because TimeoutAccessImpl does not override toString which inherited by default from scala.Function1.

akka-ci avatar Sep 08 '16 15:09 akka-ci

Comment by ktoso Tuesday Mar 22, 2016 at 16:35 GMT


I'm not quite sure it should print everything it has inside it - it's mostly very internal things (and the entire HttpRequest etc).

I assume you saw it by printing the HttpRequest somewhere - which contains the Timeout-Access header, which contains the impl...

I'd actually say that

  1. we don't want to render internals of Timeout-Access - maybe just replace with "..." and 2) perhaps we should even hide this header from being rendered, it is an implementation detail that we transmit it like that to user code.

akka-ci avatar Sep 08 '16 15:09 akka-ci

Comment by nartamonov Wednesday Mar 23, 2016 at 07:59 GMT


@ktoso, ok, I understand. But how can I retrieve value of timeout from this header? According to official documentation, "the HTTP server layer attaches a Timeout-Access header to the request, which enables programmatic customization of the timeout period and timeout response for each request individually". What I want is to extract value of timeout from header to use it later in request processing. For example, to specify timeout when ask-ing some actor. headerValueByName() directive gives me 'String', not direct access to TimeoutAccess instance.

akka-ci avatar Sep 08 '16 15:09 akka-ci

Comment by ktoso Monday Apr 04, 2016 at 00:47 GMT


The Timeout-Access header is more designed to control the timeout, not read it. So you're right that there's no clean way to access it currently. I agree it would be good to make this inspectable, even for debugging reasons etc, marking as triaged.

akka-ci avatar Sep 08 '16 15:09 akka-ci

The bad thing about that header: it fails validation in akka-http itself. We have a proxy that routes requests between different hosts, basically, we generate outgoing http requests by calling "withHost(...)" on an incoming request, preserving all origin request's info that includes Timeout-Access header. That way we receiving warning from akka on each passed request.

WARN [akka.actor.ActorSystemImpl(Proxy)] HTTP header 'Timeout-Access: ' is not allowed in requests

Maybe we are doing proxying wrong and there's more correct way? Is it possible to avoid generation of that header at all? As final measure we could set akka.actor.* logging to errors only... but that would still fail validation every time somewhere in akka :\

Discipe avatar Jan 17 '17 17:01 Discipe

There are always headers that cannot be directly proxied. You can filter it out by name. We should also just ignore internal headers instead of logging a warning.

jrudolph avatar Jan 23 '17 10:01 jrudolph

ref #911

aruediger avatar Dec 13 '17 13:12 aruediger

Any change on this issue ? We are using akka as a proxy and keep getting "HTTP header 'Timeout-Access: ' is not allowed in requests" when on server side the query is read.

To tackle the issue, I choose to rewrite this header as a raw header before sending the request to the client. But clearly the 2 fixes should be on akka side like I mention below.

/**
   * cf ****, 2 existing bugs in akka :
   *  - akka.http.impl.engine.server.HttpServerBluePrint.TimeoutAccessImpl doesn't render properly its value
   *  - akka.http.impl.engine.rendering.HttpRequestRendererFactory : Missing 'Timeout-access' case
   *
   *  Temporary solution : filter out wrong header, and replace it by a raw header
   * @param headers headers to fix
   * @return fixed headers
   */
  private def fixTimeoutAccessNotProperlyHandledInAkka(headers: Seq[HttpHeader]): Seq[HttpHeader] = {
    val headerTimeoutAccess = headers.filter(FILTER_HEADER_TIMEOUT_ACCESS_IS_WRONG)
    if (headerTimeoutAccess.nonEmpty) {
      headers
        .filterNot(FILTER_HEADER_TIMEOUT_ACCESS_IS_WRONG)
        .:+(makeHeader(H_TIMEOUT_ACCESS, headerTimeoutAccess.head.asInstanceOf[`Timeout-Access`].timeoutAccess.getTimeout().toString))
    }else{
      headers
    }
  }

fzetule-t avatar Jun 07 '22 08:06 fzetule-t

I created a branch but cannot push. Proposed fixed below (I am quite new in scala, so to be reviewed ofc)

In akka.http.impl.engine.rendering.HttpRequestRendererFactory image

code :

          case x: `Timeout-Access` =>
            render(x)
            renderHeaders(tail, hostHeaderSeen, userAgentSeen = true, transferEncodingSeen)

In akka.http.impl.engine.server.HttpServerBluePrint.TimeoutAccessImpl

image

code :

  private class TimeoutAccessImpl(request: HttpRequest, initialTimeout: Duration, requestEnd: Future[Unit],
                                  trigger:      AsyncCallback[(TimeoutAccess, HttpResponse)],
                                  materializer: Materializer, log: LoggingAdapter)
    extends AtomicReference[Future[TimeoutSetup]] with TimeoutAccess with Renderable with (HttpRequest => HttpResponse) { self =>
    import materializer.executionContext

    private var currentTimeout = initialTimeout

    def render[R <: Rendering](r: R): r.type = r ~~ "Timeout-Access" ~~ ':' ~~ ' ' ~~ currentTimeout.toString()

fzetule-t avatar Jun 07 '22 08:06 fzetule-t

To tackle the issue, I choose to rewrite this header as a raw header before sending the request to the client. But clearly the 2 fixes should be on akka side like I mention below.

No, this is not the right approach. This is a header that should not be send over the wire (because no one but akka-http understands its meaning), hence the warning.

I think we can probably just ignore any SyntheticHeader instance in the renderers in the first place. Going forward, we started to replace synthetic headers with request attributes and that's what we should also do for Timeout-Access.

jrudolph avatar Jun 07 '22 13:06 jrudolph