Project

General

Profile

Bug #4529

gerrit causes redirects to https://:80

Added by laforge 6 months ago. Updated 17 days ago.

Status:
Feedback
Priority:
Low
Assignee:
Category:
-
Target version:
-
Start date:
05/05/2020
Due date:
% Done:

50%

Spec Reference:

Description

Our cgit contains links to gerrit, see e.g. https://git.osmocom.org/osmo-bsc/commit/?id=93830d7fc46e3116a0ba59c2ebfa2d3271962077 links to https://gerrit.osmocom.org/r/I6f8be40ea54a3083f4b21ab938cc1723fc67c2ef

gerrit then produces a redirect but to the weird combination of https on port 80, which doens't work:
https://gerrit.osmocom.org:80/c/osmo-bsc/+/18017/

This was first observed after upgrading from gerrit 2.16.16 to gerrit 2.16.18.

Initial investigation seems to indicate this redirect is indeed generated by gerrit itself, not by our nginx reverse proxy.

History

#1 Updated by keith 5 months ago

From some testing with curl, this 302 Redirect response only happens if the Cookie GERRIT_UI=GWT is set in the Request

#2 Updated by keith 5 months ago

I can reproduce this by clearing all cookies for gerrit.osmocom.org, then switching to old UI, here i get an immediate redirect to port 80. There after the OP descibed problem persists.

Switching to New UI or deleting the cookie GERRIT_UI removes the problem.

#3 Updated by laforge 5 months ago

As a workaroundfor the cgit-generated URLs, let's change them as implemented in https://gerrit.osmocom.org/c/docker-playground/+/18132

#4 Updated by laforge 5 months ago

  • % Done changed from 0 to 50

after changing the cgit-generated URLs from https://gerrit.osmocom.org/r/ to https://gerrit.osmocom.org/q/ the links in the cgit pages are working again. Wouldof course still be great if the old ones kept working...

#5 Updated by keith 5 months ago

The GWT UI is even broken to the extent that Clicking "My" - > "Changes" which opens https://gerrit.osmocom.org/dashboard/self in a new tab causes this redirect bug.

In fact anything that goes through

static void toGerrit(String target, HttpServletRequest req, HttpServletResponse rsp, boolean gwt)

in UrlModule.java seems to have this erroneous redirect.

I took a quick look at changes from v2.16.16 to v2.16.18 but did not track it down

Might be a mis or unconfigured directive in local gerrit configuration?

#6 Updated by keith 5 months ago

laforge wrote:

after changing the cgit-generated URLs from https://gerrit.osmocom.org/r/ to https://gerrit.osmocom.org/q/ the links in the cgit pages are working again. Wouldof course still be great if the old ones kept working...

oh that's interesting, so /r/ links are also broken with the polygerrit-ui.

Both /r/ and /q/ links are broken if GWT UI is in use.

I'd like to see the configuration

#7 Updated by keith 5 months ago

Actually, makes sense.

UrlModule.java:92 has

serveRegex("^/r/(.+)/?$").with(DirectChangeByCommit.class);

whereas /q/ links are not matched around the above line.

I think the DirectChangeByCommit ends up calling UrlModule.toGerrit() resulting in the redirect.

Key to this, if it can be fixed through configuration, is possibly figuring out where the HttpServletRequest is getting contextPath configured from.

#8 Updated by laforge 5 months ago

thanks for investigating. I've sent you the config by private e-mail.

#9 Updated by keith 5 months ago

OK forget the nonsense I said in #4529-7

I'm assuming I'm looking at the source of the build we are using, git tag v.2.16.18

Everything that results in a redirect to

Location: https://gerrit.osmocom.org:80/
is happening because this code is called:
 static void toGerrit(String target, HttpServletRequest req, HttpServletResponse rsp, boolean gwt)
      throws IOException {
    final StringBuilder url = new StringBuilder();
    url.append(req.getContextPath());
    if (gwt) {
      url.append("/#");
    }
    url.append(target);
    rsp.sendRedirect(url.toString());
  }

so url.toString here is going to be the resource part.

Documentation for HttpServletResponse.sendRedirect(java.lang.String)
says:

This method can accept relative URLs; the servlet container must convert the relative URL to an absolute URL before sending the response to the client. If the location is relative without a leading '/' the container interprets it as relative to the current request URI. If the location is relative with a leading '/' the container interprets it as relative to the servlet container root.

Documentation for HttpServletRequest.getContextPath() says:

The path starts with a "/" character but does not end with a "/" character. For servlets in the default (root) context, this method returns ""

Either way, by sending the GWT cookie, we are sure to have a leading "/" in url, I suppose.. yes, am fumbling a bit in the dark now. I wonder if in fact it might be something to do with the nginx config? Given that I don't see bugs filed that look like this, I guess it would be good to check the nginx config too. or a trace of Headers between nginx and the gerrit httpd

Other than that, there were a number of changes in the Jetty Server from 2.16.16 to 2.16.18 and in fact this is one the few places I find a direct reference to a port "80" definition, although it's not introduced here.

I dunno.. something unexpected with the java VM version? too hard to figure out all the dependencies..

I suppose everybody uses the polygerrit UX now anyway.. With which the annoyance is minimal.

The old UX that I was still using is all but impossible now.

#11 Updated by laforge 5 months ago

On Sat, May 09, 2020 at 07:35:01PM +0000, keith [REDMINE] wrote:

Hmm... https://bugs.chromium.org/p/gerrit/issues/detail?id=12555&q=redirect&can=1

I think it's reasonably independent. Feel free to submit a new bug to gerrit.

We are using their 2.16.18 with no relevant modifications:
https://git.osmocom.org/docker-playground/tree/gerrit/Dockerfile

#12 Updated by keith 5 months ago

Unfortunately that'd be a violation of my personal policy regarding alphabet inc. I know that probably sounds illogical on many levels...

Anyway,

I'd still check a trace of nginx <-> gerrit for a GET that trigger the redirect, just to see what Headers nginx is sending.

#13 Updated by laforge 17 days ago

  • Status changed from New to Feedback
  • Assignee changed from laforge to keith
  • Priority changed from Normal to Low

Unfortuantely the problem still exists after upgrading to gerrit 3.2.3 :(

keith - how should I generate what kind of traces to help you debug?

#14 Updated by keith 17 days ago

laforge I see that now thw GWT UI is gone, and that cgit generates the /q/ urls so pretty much it's all functional.

re the trace, maybe not helpful, but the comms between the nginx and the gerrit. so a pcap on localhost I guess, of a load of the errant URL, unless nginx is on a different machine. Still I don't think it'll help much.

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)