Skip to content
Browse files

Fix mini profiler's redirect when redirecting to URLs w/ hash fragments.

Summary: Fixing mini profiler redirect and setting up arcconfig for mini profiler repo

Test Plan:
Login as admin
Go to /redirects
Create three redirects, pointing from...
  /r/t1 to https://sites.google.com/a/khanacademy.org/forge/for-khan-employees/email-transparency#TOC-Kickoff-email-from-the-start-of-our-email-transparency-experiment
  /r/t2 to https://sites.google.com/a/khanacademy.org/forge/for-khan-employees/email-transparency?monkey=bunkey&gorilla=donkey
  /r/t3 http://google.com
  /r/t4 http://localhost:8080/r/t3

...that was four redirects, tricked ya.

Now go to all three of those /r/t{n} URLs and verify that the URLs are always valid:
  in t1's case, there should be an mp-r-id querystring parameter inserted before the hash/fragment
  in t2's case, there should be an mp-r-id querystring parameter tacked onto the end of the URL
  in t3's case, there should be an mp-r-id querystring parameter tacked onto the end of the URL
  in t4's case, there should be an mp-r-id querystring parameter with *two* values, comma-separated, tacked onto the end of the URL.

TODO(kamens): unit test this sucker

Reviewers: alpert

Reviewed By: alpert

Differential Revision: http://phabricator.khanacademy.org/D4454
  • Loading branch information...
1 parent d3c9c56 commit 30c9e9029b563462395c2d8667299340b7120729 @kamens kamens committed
Showing with 31 additions and 5 deletions.
  1. +4 −0 .arcconfig
  2. +27 −5 profiler.py
View
4 .arcconfig
@@ -0,0 +1,4 @@
+{
+ "project_id": "gae-mini-profiler",
+ "conduit_uri": "http://phabricator.khanacademy.org/"
+}
View
32 profiler.py
@@ -5,6 +5,7 @@
import logging
import os
import re
+import urlparse
try:
import threading
@@ -548,6 +549,24 @@ def profiled_start_response(status, headers, exc_info = None):
@staticmethod
def headers_with_modified_redirect(environ, headers):
+ """Return headers with redirects modified to include miniprofiler id.
+
+ If this response is a redirect, we want the URL that's redirected *to*
+ to be able to display the profiler results from *this* request that's
+ being redirected *from*. We do this by adding a query string param,
+ 'mp-r-id', to the location that is being redirected to. (mp-r-id stands
+ for mini profiler redirect id.) The value of this parameter is a unique
+ identifier for the profiler results for the current request that is
+ being redirected from.
+
+ The mini profiler then knows how to use this id to display profiler
+ results for two requests: the original request that redirected and the
+ request that was served as a result of the redirect.
+
+ e.g. if this set of headers is attempting to redirect to
+ Location:http://khanacademy.org?login, the modified header will be:
+ Location:http://khanacademy.org?login&mp-r-id={current request id}
+ """
headers_modified = []
for header in headers:
@@ -561,14 +580,17 @@ def headers_with_modified_redirect(environ, headers):
request_id_chain = ",".join([match.groups()[0], request_id_chain])
# Remove any pre-existing miniprofiler redirect id
- location = header[1]
- location = reg.sub("", location)
+ url_parts = list(urlparse.urlparse(header[1]))
+ query_string = reg.sub("", url_parts[4])
# Add current request id as miniprofiler redirect id
- location += ("&" if "?" in location else "?")
- location = location.replace("&&", "&")
- location += "mp-r-id=%s" % request_id_chain
+ if query_string and not query_string.endswith("&"):
+ query_string += "&"
+ query_string += "mp-r-id=%s" % request_id_chain
+ url_parts[4] = query_string
+ # Swap in the modified Location: header.
+ location = urlparse.urlunparse(url_parts)
headers_modified.append((header[0], location))
else:
headers_modified.append(header)

0 comments on commit 30c9e90

Please sign in to comment.
Something went wrong with that request. Please try again.