feat: add option to pass redirect Location: header value as-is without encoding, decoding, or escaping

[iocanel]

Fixes #864

At the moment I have no actual way to test it, as I failed to use 1.33.1-SNAPSHOT with Jib (any helping hand is more than welcome).

[elharo]

This needs tests, and those should probably be written before the model code.

[elharo]

I see, typo in the issue number.This is for #864, not #834

[chanseokoh]

Thanks for doing this. I see there are still several places that need to be updated as well.

places that still need fixing (click to expand)

  public final String buildAuthority() {
    ...
    if (userInfo != null) {
      buf.append(CharEscapers.escapeUriUserInfo(userInfo)).append('@');

  private void appendRawPathFromParts(StringBuilder buf) {
      ...
      if (pathPart.length() != 0) {
        buf.append(CharEscapers.escapeUriPath(pathPart));

  public final String buildRelativeUrl() {
    ...
    if (fragment != null) {
      buf.append('#').append(URI_FRAGMENT_ESCAPER.escape(fragment));

  public static List<String> toPathParts(String encodedPath) {
      ...
      result.add(CharEscapers.decodeUri(sub));

  static void addQueryParams(Set<Entry<String, Object>> entrySet, StringBuilder buf) {
        ...
        String name = CharEscapers.escapeUriQuery(nameValueEntry.getKey());

  private static boolean appendParam(boolean first, StringBuilder buf, String name, Object value) {
    ...
    String stringValue = CharEscapers.escapeUriQuery(value.toString());

[elharo]

Have you considered whether a different class or a subclass might be an option instead?

[chanseokoh]

Thanks! I will start testing this on the Jib side.

[chanseokoh]

Thanks again.

[chanseokoh]

I've tested the new behavior with TestWebServer.

  public static void main(String[] args) throws Exception {
    String url1 = "?id=301&_auth_=exp=1572285389~hmac=f0a387f0";
    String url2 = "?id=302&Signature=2wYOD0a%2BDAkK%2F9lQJUOuIpYti8o%3D&Expires=1569997614";
    String url3 = "?id=303&_auth_=exp=1572285389~hmac=f0a387f0";
    String url4 = "?id=307&Signature=2wYOD0a%2BDAkK%2F9lQJUOuIpYti8o%3D&Expires=1569997614";
//  String url5 = "?code=308&_auth_=exp=1572285389~hmac=f0a387f0";

    String redirect301 = "HTTP/1.1 301 Moved Permanently\nLocation: " + url1 + "\nContent-Length: 0\n\n";
    String redirect302 = "HTTP/1.1 302 Found\nLocation: " + url2 + "\nContent-Length: 0\n\n";
    String redirect303 = "HTTP/1.1 303 See Other\nLocation: " + url3 + "\nContent-Length: 0\n\n";
    String redirect307 = "HTTP/1.1 307 Temporary Redirect\nLocation: " + url4 + "\nContent-Length: 0\n\n";
//  String redirect308 = "HTTP/1.1 308 Permanent Redirect\nLocation: " + url5 + "\nContent-Length: 0\n\n";
    String ok200 = "HTTP/1.1 200 OK\nContent-Length:12\n\nHello World!";
    List<String> responses = Arrays.asList(redirect301, redirect302, redirect303, redirect307, ok200);

    try (TestWebServer server = new TestWebServer(false, responses, 1)) {
      new ApacheHttpTransport().createRequestFactory()
          .buildGetRequest(new GenericUrl(server.getEndpoint()))
          .setUseRawRedirectUrls(true).execute().disconnect();
      System.out.println(server.getInputRead());
    }
  }

The output is the following, and I can see that all the redirects preserved the raw URL values.

GET / HTTP/1.1
Accept-Encoding: gzip
User-Agent: Google-HTTP-Java-Client/1.33.1-SNAPSHOT (gzip)
Host: 0.0.0.0:41115
Connection: Keep-Alive
GET /?id=301&_auth_=exp=1572285389~hmac=f0a387f0 HTTP/1.1
Accept-Encoding: gzip
User-Agent: Google-HTTP-Java-Client/1.33.1-SNAPSHOT (gzip)
Host: 0.0.0.0:41115
Connection: Keep-Alive
GET /?id=302&Signature=2wYOD0a%2BDAkK%2F9lQJUOuIpYti8o%3D&Expires=1569997614 HTTP/1.1
Accept-Encoding: gzip
User-Agent: Google-HTTP-Java-Client/1.33.1-SNAPSHOT (gzip)
Host: 0.0.0.0:41115
Connection: Keep-Alive
GET /?id=303&_auth_=exp=1572285389~hmac=f0a387f0 HTTP/1.1
Accept-Encoding: gzip
User-Agent: Google-HTTP-Java-Client/1.33.1-SNAPSHOT (gzip)
Host: 0.0.0.0:41115
Connection: Keep-Alive
GET /?id=307&Signature=2wYOD0a%2BDAkK%2F9lQJUOuIpYti8o%3D&Expires=1569997614 HTTP/1.1
Accept-Encoding: gzip
User-Agent: Google-HTTP-Java-Client/1.33.1-SNAPSHOT (gzip)
Host: 0.0.0.0:41115
Connection: Keep-Alive

I also added a basically same test on Jib, and it also works the same way as expected. So, at least this seems to work end-to-end.

(I noticed 308 Permanent Redirect isn't automatically redirected, and this may be a bug. Filed #873.)

@iocanel however, I haven't tested Jib against actual OpenShift and Quay servers.

[iocanel]

@chanseokoh: Thanks! If you have a branch of jib working with this PR, I could possilby use it and test it against Openshift and Quay.

[chanseokoh]

@iocanel the branch is already there. Check this out: #871 (comment)

(The comment that I linked is hidden somewhere in our PR conversation history above.)

[chanseokoh]

Hey @iocanel, a few things to fix.

[iocanel]

@iocanel I think this is working. Have you had a chance to test this against OpenShift and Quay from Jib?

Tried the fix against registry.redhat.io and it seems that it works fine.

[chanseokoh]

Thanks for verifying that. The code changes make sense to me and I think it works.

@yoshi-java @chingor13 please review this.

dismissing initial stale review

[iocanel]

I think that all points have been covered.

[chanseokoh]

@chingor13 @codyoss friendly ping. I'd like to get this moving.

[chanseokoh]

@chingor13 @codyoss @Yoshi-Java can we aim for having this fix for the next release? This is pretty significant stuff for us.

[codyoss]

LGMT, thanks for all of your iterations on this.

[iocanel]

@chanseokoh any idea on when this PR is going to be part of a release?

[chanseokoh]

I think this will be in the next release. But I don't know when they will make a release. @codyoss?

[codyoss]

I am going to try to get a couple more things in this release. But if they are not going to land I will make a note to try to cut a release tomorrow regardless. @chanseokoh @iocanel