dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.11k stars 1.56k forks source link

HttpClientRequest.isRedirect returns false in case of redirect #31962

Open sgrekhov opened 6 years ago

sgrekhov commented 6 years ago

According to the HttpClientRequest.isRedirect this getter should return true if status code is one of the normal redirect codes. But the following code demonstrates that this property returns false anyway

import "dart:io";
import "dart:convert";

var localhost = InternetAddress.LOOPBACK_IP_V4.address;

test(String method, int statusCode) async {
  bool redirected = false;
  String helloWorld = "Hello world";
  HttpServer server = await HttpServer.bind(localhost, 0);
  Uri redirectUri = Uri.parse("http://${localhost}:${server.port}/yyy");
  server.listen((HttpRequest request) {
    if (request.uri.path == "/xxx") {
      request.response.redirect(redirectUri, status: statusCode);
      redirected = true;
    } else if (request.uri.path == "/yyy") {
      print("Redirected: $redirected"); // Redirected: true
      request.response.write(helloWorld);
      request.response.close();
      server.close();
    }
  });

  HttpClient client = new HttpClient();
  client.open(method, localhost, server.port, "/xxx")
      .then((HttpClientRequest request) {
    print("request.followRedirects: ${request.followRedirects}"); // followRedirects: true
    return request.close();
  }).then((HttpClientResponse response) {
    print("isRedirect: ${response.isRedirect}"); // isRedirect: false (!!!)
    print(response.redirects[0].statusCode); // One of 301, 302, 303, 307
    response.transform(UTF8.decoder).listen((content) {
      print(content); // Hello world
    });
  });
}

main() {
  test("get", HttpStatus.MOVED_PERMANENTLY);
  test("get", HttpStatus.FOUND);
  test("get", HttpStatus.MOVED_TEMPORARILY);
  test("get", HttpStatus.SEE_OTHER);
  test("get", HttpStatus.TEMPORARY_REDIRECT);

  test("head", HttpStatus.MOVED_PERMANENTLY);
  test("head", HttpStatus.FOUND);
  test("head", HttpStatus.MOVED_TEMPORARILY);
  test("head", HttpStatus.SEE_OTHER);
  test("head", HttpStatus.TEMPORARY_REDIRECT);
}
eturk1 commented 6 years ago

any success on this 6 months in @sgrekhov, @rmacnak-google ?

I'm trying to redirect, get response.statusCode=302, but I can't get the client to follow the redirect, send the POST, get the proper location page

sortie commented 5 years ago

@sgrekhov I think this issue is mistaken. isRedirect is only true if the response is a redirect, but whenever followRedirects is true, redirects are followed and you instead get the redirected response which is not a redirect unlike the original response. You should instead test that response.redirects contains the redirects that you expected. Do you agree? If so, then the tests/co19_2/src/LibTest/io/HttpClientResponse/isRedirect_A01_t02.dart test should be updated accordingly (should I file an issue in the co19 repository?).

I can also clarify the documentation for isRedirect by saying it will never be true if followRedirects is true.

@eturk1 You should send HTTP 303 See Other instead of HTTP 302 Found when redirecting POST requests. This is the logic for deciding whether to follow the redirect in the Dart SDK:

  bool get isRedirect {
    if (_httpRequest.method == "GET" || _httpRequest.method == "HEAD") {
      return statusCode == HttpStatus.movedPermanently ||
          statusCode == HttpStatus.found ||
          statusCode == HttpStatus.seeOther ||
          statusCode == HttpStatus.temporaryRedirect;
    } else if (_httpRequest.method == "POST") {
      return statusCode == HttpStatus.seeOther;
    }
    return false;
  }