Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
akka-ci opened this issue Sep 8, 2016 · 9 comments
Open
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted help wanted Identifies issues that the core team will likely not have time to work on nice-to-have (low-prio) Tasks which make sense, however are not very high priority, feel free to help out!

Comments

@akka-ci
Copy link

akka-ci commented Sep 8, 2016

Issue by nartamonov
Tuesday Mar 22, 2016 at 16:00 GMT
Originally opened as akka/akka#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 akka-ci added this to the 2.4.x milestone Sep 8, 2016
@akka-ci akka-ci added nice-to-have (low-prio) Tasks which make sense, however are not very high priority, feel free to help out! t:http labels Sep 8, 2016
@akka-ci
Copy link
Author

akka-ci commented Sep 8, 2016

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
Copy link
Author

akka-ci commented Sep 8, 2016

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
Copy link
Author

akka-ci commented Sep 8, 2016

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.

@ktoso ktoso added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted and removed t:http labels Sep 8, 2016
@ktoso ktoso removed this from the 2.4.x milestone Sep 12, 2016
@Discipe
Copy link

Discipe commented Jan 17, 2017

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 :\

@jrudolph
Copy link
Contributor

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.

@aruediger
Copy link
Contributor

ref #911

@fzetule-t
Copy link

fzetule-t commented Jun 7, 2022

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
Copy link

fzetule-t commented Jun 7, 2022

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()

@jrudolph
Copy link
Contributor

jrudolph commented Jun 7, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted help wanted Identifies issues that the core team will likely not have time to work on nice-to-have (low-prio) Tasks which make sense, however are not very high priority, feel free to help out!
Projects
None yet
Development

No branches or pull requests

6 participants