LNT: fix broken relative redirect

[tnfchris]

Summary

RFC 2616 had at some point an ambiguous statement in it that made it
look like relative URIs was not allowed in the Location header. This RFC is
now outdated and superseded with RFC 7231

Some implementations like Flask implemented their response object
based on this mistake. This ambiguity was later corrected in updated
RFC 7231 but Flask has kept the old implementation. Which means to get
correct standards compliant redirects we need to override the default
responder and tell it not to force absolute URIs.

Former wording of the RFC:

The Location response-header field is used to redirect the recipient to a location other than the Request-URI for completion of the request or identification of a new resource. For 201 (Created) responses, the Location is that of the new resource which was created by the request. For 3xx responses, the location SHOULD indicate the server's preferred URI for automatic redirection to the resource. The field value consists of a single absolute URI.

Location       = "Location" ":" absoluteURI

Current wording

The "Location" header field is used in some responses to refer to a
specific resource in relation to the response.  The type of
relationship is defined by the combination of request method and
status code semantics.

  Location = URI-reference

[1] https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
[2] https://tools.ietf.org/html/rfc7231#section-7.1.2

[thopre]

In the description:

has at some point -> had at some point

Would you happen to know the former wording? If not don't bother searching but if yes maybe you could quote it in the description

lnt/server/ui/globals.py

45

I don't quite understand this line but how about:

adhering to a typo the out-dated -> a misunderstanding of an ambiguous statement in a former version of RFC 2616

47

by -> but
doesn't -> does not

[tnfchris]

Updated comment

[thopre]

LGTM with the contraction fix (doesn't -> does not)

lnt/server/ui/globals.py

48

doesn't -> does not

lnt/server/ui/globals.py

 """
 
 import flask
+from flask import Response
 
 import lnt.server.ui.util
 

     except Exception:
         return False
 
+class fixed_location_response(Response):
+    autocorrect_location_header = False
+
+def v4_redirect(*args, **kwargs):
+    """
+    Like redirect but can be used to allow relative URL redirection.
+    Flask's default response makes URLs absolute before putting in the
+    Location header due to adhering to the out-dated RFC 2616 which has
+    been superseded by RFC 7231.
+
+    The RFC outdated in 2018 but Flask still does not implement the new
+    standard.
+    """
+    return flask.redirect(*args, Response=fixed_location_response, **kwargs)
 
 def register(env):
     # Add some normal Python builtins which can be useful in templates.

     env.globals.update(
         db_url_for=db_url_for,
         v4_url_for=v4_url_for,
+        v4_redirect=v4_redirect,
         v4_url_available=v4_url_available)

lnt/server/ui/regression_views.py

 
 from lnt.server.ui.decorators import v4_route
 import lnt.server.reporting.analysis
-from lnt.server.ui.globals import v4_url_for
+from lnt.server.ui.globals import v4_url_for, v4_redirect
 from lnt.server.ui.views import ts_data
 
 from lnt.util import logger

             request.form['btn'] == "Create New Regression":
         regression, _ = new_regression(session, ts, form.field_changes.data)
         flash("Created " + regression.title, FLASH_SUCCESS)
-        return redirect(v4_url_for(".v4_regression_list",
+        return v4_redirect(v4_url_for(".v4_regression_list",
                         highlight=regression.id))
     if request.method == 'POST' and request.form['btn'] == "Ignore Changes":
         msg = "Ignoring changes: "

 
         session.commit()
         flash("Created: " + new_regress.title, FLASH_SUCCESS)
-        return redirect(v4_url_for(".v4_regression_detail", id=new_regress.id))
+        return v4_redirect(v4_url_for(".v4_regression_detail", id=new_regress.id))
     # Delete requested regressions.
     if request.method == 'POST' and \
             request.form['merge_btn'] == "Delete Regressions":

             session.delete(reg)
         session.commit()
         flash(' Deleted: '.join(titles), FLASH_SUCCESS)
-        return redirect(v4_url_for(".v4_regression_list", state=state_filter))
+        return v4_redirect(v4_url_for(".v4_regression_list", state=state_filter))
 
     q = session.query(ts.Regression)
     title = "All Regressions"

         regression_info.state = form.state.data
         session.commit()
         flash("Updated " + regression_info.title, FLASH_SUCCESS)
-        return redirect(v4_url_for(".v4_regression_list",
+        return v4_redirect(v4_url_for(".v4_regression_list",
                         highlight=regression_info.id,
                         state=int(form.edit_state.data)))
     if request.method == 'POST' and \

         lnt.server.db.fieldchange.rebuild_title(session, ts, regression_info)
         session.commit()
         flash("Split " + second_regression.title, FLASH_SUCCESS)
-        return redirect(v4_url_for(".v4_regression_list",
+        return v4_redirect(v4_url_for(".v4_regression_list",
                         highlight=second_regression.id,
                         state=int(form.edit_state.data)))
     if request.method == 'POST' and request.form['save_btn'] == "Delete":

         session.delete(regression_info)
         session.commit()
         flash("Deleted " + title, FLASH_SUCCESS)
-        return redirect(v4_url_for(".v4_regression_list",
+        return v4_redirect(v4_url_for(".v4_regression_list",
                         state=int(form.edit_state.data)))
     form.field_changes.choices = list()
     form.state.default = regression_info.state

     logger.info("Manually created new regressions: {}".format(regression.id))
     flash("Created " + regression.title, FLASH_SUCCESS)
 
-    return redirect(v4_url_for(".v4_regression_detail", id=regression.id))
+    return v4_redirect(v4_url_for(".v4_regression_detail", id=regression.id))

lnt/server/ui/views.py

 from lnt.server.reporting.analysis import ComparisonResult, calc_geomean
 from lnt.server.ui import util
 from lnt.server.ui.decorators import frontend, db_route, v4_route
-from lnt.server.ui.globals import db_url_for, v4_url_for
+from lnt.server.ui.globals import db_url_for, v4_url_for, v4_redirect
 from lnt.server.ui.util import FLASH_DANGER, FLASH_SUCCESS, FLASH_INFO
 from lnt.server.ui.util import PrecomputedCR
 from lnt.server.ui.util import baseline_key, convert_revision

 
 @frontend.route('/favicon.ico')
 def favicon_ico():
-    return redirect(url_for('.static', filename='favicon.ico'))
+    return v4_redirect(url_for('.static', filename='favicon.ico'))
 
 
 @frontend.route('/select_db')

     else:
         if '/' in path[1:]:
             new_path += "/" + path.split("/", 2)[2]
-    return redirect(request.script_root + new_path)
+    return v4_redirect(request.script_root + new_path)
 
 #####
 # Per-Database Routes

     """Compatibility url that hardcodes testsuite to 'nts'"""
     if request.method == 'GET':
         g.testsuite_name = 'nts'
-        return redirect(v4_url_for('.v4_submitRun'))
+        return v4_redirect(v4_url_for('.v4_submitRun'))
 
     # This route doesn't know the testsuite to use. We have some defaults/
     # autodetection for old submissions, but really you should use the full

         .filter(ts.Run.machine_id == machine_id) \
         .order_by(ts.Run.start_time.desc()) \
         .first()
-    return redirect(v4_url_for('.v4_run', id=run.id, **request.args))
+    return v4_redirect(v4_url_for('.v4_run', id=run.id, **request.args))
 
 
 @v4_route("/machine/<int:machine_id>/compare")

         .order_by(ts.Run.start_time.desc()) \
         .first()
 
-    return redirect(v4_url_for('.v4_run', id=machine_1_run.id,
+    return v4_redirect(v4_url_for('.v4_run', id=machine_1_run.id,
                                compare_to=machine_2_run.id))
 
 

 
     # If we found one, redirect to it's report.
     if matched_run is not None:
-        return redirect(db_url_for(".v4_run", testsuite_name=tag,
+        return v4_redirect(db_url_for(".v4_run", testsuite_name=tag,
                                    id=matched_run.id))
 
     # Otherwise, report an error.

             session.commit()
 
             flash("Baseline {} updated.".format(baseline.name), FLASH_SUCCESS)
-        return redirect(v4_url_for(".v4_order", id=id))
+        return v4_redirect(v4_url_for(".v4_order", id=id))
 
     try:
         baseline = session.query(ts.Baseline) \

     flash("Baseline set to " + base.name, FLASH_SUCCESS)
     flask.session[baseline_key(ts.name)] = id
 
-    return redirect(get_redirect_target())
+    return v4_redirect(get_redirect_target())
 
 
 @v4_route("/all_orders")

             run.machine.id, test_id, value)
         plot_number += 1
 
-    return redirect(v4_url_for(".v4_graph", **args))
+    return v4_redirect(v4_url_for(".v4_graph", **args))
 
 
 BaselineLegendItem = namedtuple('BaselineLegendItem', 'name id')

     kwargs.update(request.args)
 
     graph_url = v4_url_for('.v4_graph', **kwargs)
-    return redirect(graph_url)
+    return v4_redirect(graph_url)
 
 
 @v4_route("/graph")

     extra_args.pop("month", None)
     extra_args.pop("day", None)
 
-    return redirect(v4_url_for(".v4_daily_report",
+    return v4_redirect(v4_url_for(".v4_daily_report",
                                year=date.year, month=date.month, day=date.day,
                                **extra_args))
 

             flask.json.dump(config, f, indent=2)
 
         # Redirect to the summary report.
-        return redirect(db_url_for(".v4_summary_report"))
+        return v4_redirect(db_url_for(".v4_summary_report"))
 
     config_path = get_summary_config_path()
     if os.path.exists(config_path):