redstone-dart / redstone

A metadata driven microframework for Dart.
http://redstone-dart.github.io/redstone
MIT License
342 stars 42 forks source link

chain.forward not forwarding on v0.6 #123

Closed cgarciae closed 9 years ago

cgarciae commented 9 years ago

I am doing a test for some other feature/bug concerning redirection, I found some other bug, chain.forward is not forwarding. Here are my routes

@Group('/test_wrapper')
class TestRedirectWrapper {
  @Route("/redirect")
  @Wrap("REDIRECT")
  testWrapperRedirect() {
    print("A");
    return chain.forward('/test_wrapper/b');
  }

  @Route("/b")
  @Wrap("response")
  testWrapperB() {
    print("B");
    return "target executed";
  }
}

when the test calls /test_wrapper/redirect I get A printed on the screen forever, as if its "forwarding" to itself, it is very weird.

luizmineo commented 9 years ago

The router library is responsible for handling http requests, and it contains the implementation of the Chain interface. Take a look at the forward method here

It seems that this method is handling urls completely wrong:

var newUrl = req.requestedUri.resolve(url);

It should do something like this:

var newUrl = url.startsWith('/') ? Uri.parse(url) : _joinUrl(req.requestedUri.toString(), url);
cgarciae commented 9 years ago

Apart from small modifications, this code produces the same behavior: even if the new URL is the correct one, it triggers the old route.

Future<shelf.Response> forward(String url,
      {Map<String, String> headers}) async {
    var req = currentContext.request;
    var newUrl = url.startsWith('/') ? req.requestedUri.resolve(url) : Router._joinUrl(req.requestedUri.toString(), url);
    var shelfReqCtx = new Map.from(req.attributes);
    var newReq = new shelf.Request("GET", newUrl
      ,headers: headers, context: shelfReqCtx);

    return _forwardShelfHandler(newReq);
  }

My intuition is that _forwardShelfHandler but the inner workings of shelf are a mystery to me.

cgarciae commented 9 years ago

I've created the branch #123 with my changes if you want to look at them.

luizmineo commented 9 years ago

_forwardShelfHandler is just a shelf handler that creates a new chain object, and executes it with the new request.

The problem is that the chain implementation is always looking to the original HttpRequest object when searching for handlers that match the request, instead of looking to the current shelf request.

I've made some changes that fix this behaviour, and it seems to work fine now, although it needs more proper tests.

cgarciae commented 9 years ago

@luizmineo thanks a lot. I am using the redstone MVC plugin but had to implement some client scripts for redirecting, hopefully with this I won't need to.