akka / akka-http

The Streaming-first HTTP server/module of Akka
https://doc.akka.io/libraries/akka-http/current/
Other
1.34k stars 594 forks source link

Add automatic redirection handling #195

Open ktoso opened 8 years ago

ktoso commented 8 years ago

Issue by jrudolph Monday Sep 29, 2014 at 08:00 GMT Originally opened as https://github.com/akka/akka/issues/15990


The high-level client-side API should support some form of configurable redirection handling.

Several things need to be taken into account:

Before starting on this, it would make sense to go through the mailing list / spray tickets to find further cases we might need to support.

A first solution could only support redirections to the same domain, where keeping certain headers may be safer (but: cookies with path constraints set).

Maybe also consider how browsers deal with redirections.

This is part of the bigger initiative to support a high-level HTTP client interface as tracked as #16856.

/cc @sirthias

ktoso commented 8 years ago

Comment by huntc Thursday Mar 05, 2015 at 02:37 GMT


I've just stumbled across the need for this and will have to of course hand craft it. I just thought that you'd like to know of at least one person who needs this functionality.

ktoso commented 8 years ago

Comment by francisdb Sunday Jul 19, 2015 at 08:23 GMT


Another case to handle would be redirect loops

ktoso commented 8 years ago

Comment by CptnKirk Wednesday Oct 14, 2015 at 23:26 GMT


Also +1 for this feature. Did tracking of this issue get lost? Was 1.0-RC4, now I see it has no milestone. Yet I don't see that anyone has modified the milestone since sirthias on Jun 24.

ktoso commented 8 years ago

Comment by coreyauger Wednesday Nov 18, 2015 at 23:26 GMT


Also +1 from me.

ktoso commented 8 years ago

Comment by ktoso Wednesday Feb 24, 2016 at 14:18 GMT


Logging a +1 :point_up: February 24, 2016 3:11 PM

We'd welcome contributions of this feature, anyone interested in helping out? :-)

ktoso commented 8 years ago

Comment by RomanIakovlev Monday Feb 29, 2016 at 13:43 GMT


I'd like to try to help with this, since I need it in my project. However, I'm totally new to akka source code, so I'd definitely need some initial guidance. What would be the best channel to ask questions related to this?

ktoso commented 8 years ago

Comment by ktoso Monday Feb 29, 2016 at 13:57 GMT


Thank a lot for offering help on that :-) Best discussed here actually as it's nice and async, or gitter (however this week the team is mostly offline, planning the next months of development, so we may be a bit slow to respond - sorry about that).

ktoso commented 8 years ago

Comment by RomanIakovlev Monday Feb 29, 2016 at 18:10 GMT


Thanks, I'd be happy to help. Just not sure if I'm up to this task, but one never knows unless one tries. :)

Ok, here are the initial questions.

  1. API overview. I imagine adding a couple of properties to the akka.http.scaladsl.model.HttpRequest, like followRedirects: Boolean = false and maxRedirects: Int = 1. Or maybe wrap these two in a single case class. What do you think?
  2. Where the actual redirect handling code should live? So far my best guess is akka.http.impl.engine.client.OutgoingConnectionBlueprint, but I'm in doubts.

This is just to get my thoughts straight before I can begin even thinking of possible solutions.

ktoso commented 8 years ago

Comment by jamesmulcahy Tuesday Mar 01, 2016 at 00:45 GMT


@RomanIakovlev You probably also need to consider adding an option to decide which headers are re-submitted when the re-direct is followed. Spray used to drop the headers, which is apparently an appealing approach to some, but doesn't suit all scenarios. See spray/spray#1045.

ktoso commented 8 years ago

Comment by RomanIakovlev Thursday Mar 17, 2016 at 20:43 GMT


I've started working on this issue, and I'm sort of stuck on some questions I can't resolve myself.

My overall idea is to create a GraphStage[BidiShape[HttpRequest, HttpRequest, HttpResponse, HttpResponse]], which will sit on top of the OutgoingConnectionBlueprint. This new stage will keep track of incoming request (In1) and forward them downstream (Out1). When response comes (In2), the stage checks if the response is redirect, and, if it is, sends the new redirected request (Out1). When the "real" response will come, it's forwarded to requester (Out2). I don't want to send the pull request so far, but, if you wish, you can have a look at the current implementation in branch wip-15990-http-client-redirect-RomanIakovlev in my forked Akka repo at https://github.com/RomanIakovlev/akka/

This implementation fails at one of the tests in akka.http.impl.engine.client.HighLevelOutgoingConnectionSpec, concretely on the "catch response stream truncation" one. It's not completely clear to me what's the expected behavior for that test, so maybe someone can clarify?

Besides that, I have 2 questions about this approach in general.

First of all, do you think it makes sense? I mean, the GraphStage[BidiShape] part of it. Maybe there's a better way?

Secondly, if the BidiShape is the way to go, is it supposed to support multiple requests in flight? If yes, then how received response can be mapped to an incoming request? I need this correspondence to be able to copy the headers to the redirected request from the original request (and maybe for something else that I don't know yet). If only single request in flight can be supported, then I'm not sure how the pipelining will be dealt with.

I hope my questions make sense. I'm on vacations now and will be able to dedicate more time to this issue for the next week, especially if I'll have some guidance from the Akka core team. :)

ktoso commented 8 years ago

Comment by RomanIakovlev Friday Mar 18, 2016 at 12:00 GMT


Another thought has just occurred to me. The existing OutgoungConnectionBlueprint does support pipelining, obviously, but, as far as I can tell, it also guarantees the order of responses to be the same as the order of requests. Is it correct? If yes, then I can rely on this fact to get correspondence between requests and responses in my redirect supporting graph stage to support copying certain headers into redirect requests. Would that work?

ktoso commented 8 years ago

Comment by ktoso Friday Jul 08, 2016 at 12:45 GMT


This had some progress in https://github.com/akka/akka/issues/20135 but we decided together that it wasn't quite there yet. Would be awesome to see it be picked up again (@RomanIakovlev again perhaps if he has time?)

ktoso commented 8 years ago

Comment by gaydenko Thursday Aug 18, 2016 at 09:07 GMT


Have you some workaround, please? It is a hard stopper.

ktoso commented 8 years ago

Comment by ktoso Thursday Aug 18, 2016 at 09:09 GMT


Here's the options: a) help us deliver this feature - contribute! b) follow the redirect manually. c) use a different http client. Play's WS is pretty good.

ktoso commented 8 years ago

Comment by gaydenko Thursday Aug 18, 2016 at 10:45 GMT


Thanks. I know the a) is the best for akka, but resources are limited at the moment resulting in preferring the b).

ktoso commented 8 years ago

Comment by drewhk Friday Aug 19, 2016 at 11:37 GMT


Take a look at this for implementing retries: https://github.com/akka/akka-stream-contrib/pull/26

ktoso commented 8 years ago

Comment by drewhk Friday Aug 19, 2016 at 11:37 GMT


currently in contrib, but seems like useful for many things so we might want to migrate it to akka-streams and consider adding it to the HTTP client API somehow.

jrudolph commented 8 years ago

Here's a stub for redirection support and how things could be wired:

package akka.http

import akka.actor.ActorSystem
import akka.http.scaladsl.Http
import akka.http.scaladsl.model.headers.Location
import akka.http.scaladsl.model.{ HttpMethods, HttpRequest, HttpResponse, StatusCodes }
import akka.stream.ActorMaterializer

import scala.concurrent.{ ExecutionContext, Future }

object RichHttpClient {
  type HttpClient = HttpRequest ⇒ Future[HttpResponse]

  def redirectOrResult(singleRequest: HttpClient)(response: HttpResponse): Future[HttpResponse] =
    response.status match {
      case StatusCodes.Found | StatusCodes.MovedPermanently | StatusCodes.SeeOther ⇒
        val newUri = response.header[Location].get.uri

        // TODO: add debug logging

        // change to GET method as allowed by https://tools.ietf.org/html/rfc7231#section-6.4.3
        // TODO: keep HEAD if the original request was a HEAD request as well?
        // TODO: do we want to keep something of the original request like custom user-agents, cookies
        //       or authentication headers?
        singleRequest(HttpRequest(method = HttpMethods.GET, uri = newUri))
      // TODO: what to do on an error? Also report the original request/response?

      // TODO: also handle 307, which would require resending POST requests
      case _ ⇒ Future.successful(response)
    }

  def httpClientWithRedirect(client: HttpClient)(implicit ec: ExecutionContext): HttpClient = {
    lazy val redirectingClient: HttpClient =
      req ⇒ client(req).flatMap(redirectOrResult(redirectingClient)) // recurse to support multiple redirects

    redirectingClient
  }
}

object SingleRequestWithRedirect extends App {
  implicit val system = ActorSystem()
  import system.dispatcher
  implicit val mat = ActorMaterializer()

  val simpleClient = Http().singleRequest(_: HttpRequest)
  val redirectingClient = RichHttpClient.httpClientWithRedirect(simpleClient)

  val request = HttpRequest(uri = "http://goggle.de")

  simpleClient(request).onComplete(res ⇒ println(s"Without redirection: $res"))
  redirectingClient(request).onComplete(res ⇒ println(s"With redirection: $res"))
}

I added lots of TODOs where a general solution would have to actually support solutions.

akashppatel commented 7 years ago

Thank you @jrudolph , for sharing general solution it helped me :)

muller commented 7 years ago

To help answering the TODO: do we want to keep... I like the approach in http://docs.python-requests.org/en/master/user/quickstart/#redirection-and-history where the HttpClient is HttpRequest ⇒ Future[List[HttpResponse]] instead of a single HttpResponse

ktoso commented 7 years ago

The low level in Akka is like that actually. Since Flow[Request, Response] means there can be multiple responses, we simply don't need the List as it's already represented by how Flows work. I know your commend likely relates to the singleRequest API though.

gaydenko commented 7 years ago

BTW, I guess recursion must be limited. I use this workaround:

  private val maxRedirCount = 20
  def httpRequire(req: HttpRequest, count: Int = 0)(implicit system: ActorSystem, mat: Materializer): Future[HttpResponse] = {
    implicit val ec: ExecutionContext = system.dispatcher

    Http().singleRequest(req).flatMap { resp =>
      resp.status match {
        case StatusCodes.Found => resp.header[headers.Location].map { loc =>
          val locUri = loc.uri
          val newUri = req.uri.copy(scheme = locUri.scheme, authority = locUri.authority)
          val newReq = req.copy(uri = newUri)
          if (count < maxRedirCount) httpRequire(newReq, count + 1) else Http().singleRequest(newReq)
        }.getOrElse(throw new RuntimeException(s"location not found on 302 for ${req.uri}"))
        case _ => Future(resp)
      }
    }
  }
usamec commented 7 years ago

val newUri = req.uri.copy(scheme = locUri.scheme, authority = locUri.authority)

what is the reason for only replacing scheme and authority part of the URI?

gaydenko commented 7 years ago

@usamec It does work at my use cases.

usamec commented 7 years ago

@gaydenko And yet it does not work on example from wikipedia: https://en.wikipedia.org/wiki/HTTP_301 (and does not handle 301 status code).

I would copy the whole URI and handle all the 30x status codes.

gaydenko commented 7 years ago

@usamec You can modify that workaround the way you want. In common case there isn't single rule all redirectors follow to, and I was just focused on the problem in hands. Again, at my case those few line of code do the job. You can use val newReq = req.copy(uri = locUri) if it does work for you.

manonthegithub commented 7 years ago

@jrudolph

val newUri = response.header[Location].get.uri

please note that this won't work for the case when uri in location is relative and you don't use host connection pool (just encountered this case).

Also, as I understand docs, you forgot to dicard redirect response's bytes so I assume, it should look like:

response.discardEntityBytes().future().flatMap(_ =>
   singleRequest(HttpRequest(method = HttpMethods.GET, uri = newUri))
)
vvasuki commented 7 years ago

A slightly modified version of @jrudolph 's code (only new thing added: discardEntityBytes):

object RichHttpClient {
  type HttpClient = HttpRequest ⇒ Future[HttpResponse]

  def redirectOrResult(client: HttpClient)(response: HttpResponse)(implicit materializer: Materializer): Future[HttpResponse] =
    response.status match {
      case StatusCodes.Found | StatusCodes.MovedPermanently | StatusCodes.SeeOther ⇒
        val newUri = response.header[Location].get.uri
        // Always make sure you consume the response entity streams (of type Source[ByteString,Unit]) by for example connecting it to a Sink (for example response.discardEntityBytes() if you don’t care about the response entity), since otherwise Akka HTTP (and the underlying Streams infrastructure) will understand the lack of entity consumption as a back-pressure signal and stop reading from the underlying TCP connection!
        response.discardEntityBytes()
        // TODO: add debug logging

        // change to GET method as allowed by https://tools.ietf.org/html/rfc7231#section-6.4.3
        // TODO: keep HEAD if the original request was a HEAD request as well?
        // TODO: do we want to keep something of the original request like custom user-agents, cookies
        //       or authentication headers?
        client(HttpRequest(method = HttpMethods.GET, uri = newUri))
      // TODO: what to do on an error? Also report the original request/response?

      // TODO: also handle 307, which would require resending POST requests
      case _ ⇒ Future.successful(response)
    }

  def httpClientWithRedirect(client: HttpClient)(implicit ec: ExecutionContext, materializer: Materializer): HttpClient = {
    lazy val redirectingClient: HttpClient =
      req ⇒ client(req).flatMap(redirectOrResult(redirectingClient)) // recurse to support multiple redirects

    redirectingClient
  }
}
  final implicit val materializer: ActorMaterializer = ActorMaterializer(ActorMaterializerSettings(context.system))

  private val simpleClient: HttpRequest => Future[HttpResponse] = Http(context.system).singleRequest(_: HttpRequest)
  private val redirectingClient: HttpClient = RichHttpClient.httpClientWithRedirect(simpleClient)
val responseFuture = redirectingClient(HttpRequest(uri = uri))

Available as: "com.github.sanskrit-coders" % "scala-utils_2.12" % "0.1"

qstyler commented 4 years ago

Hello everyone from 2020. Was that problem solved?

nmolenaar commented 3 years ago

Hello everyone from 2021. Was that problem solved?

spaniakos commented 3 years ago

if anyone has a problem with the redirects in laravel, do check your .htaccess and see if you have any 301 redirect like: RewriteCond %{REQUEST_FILENAME} !-d RewriteRule ^(.*)/$ /$1 [L,R=301]

As redirect in akka is not following though, this will terminate the request and wan't process the redirect.

randers-bit commented 1 year ago

Hello everyone from 2023. Was that problem solved?