Permanent link on /exonerator.html? is http

When results are displayed for a a given IP in ExoneraTor, the permanent link section gives the link in HTTP.

Although the endpoint redirects to HTTPS, it's still desirable to show the HTTPS link in the first place. [Screenshot-attached]

Example: https://metrics.torproject.org/exonerator.html?ip=62.138.7.171&timestamp=2018-09-20&lang=en

Trac:
Username: modik

added component::metrics/exonerator owner::metrics-team priority::medium reporter::modik resolution::fixed severity::normal status::closed type::defect labels

Trac:
Username: modik

Trac:
Username: modik

Good catch! This is indeed not ideal.

It's even a tiny bit worse than described above: we also include the http link in other places, for example, when an IP address was not found but nearby IP addresses in the same /24 have possible hits. Try searching for .170 and look at the HTML sources:

            <div class="panel-body">
              <p>We did not find IP address 62.138.7.170 on or within a day of 2018-09-20. But we did find other IP addresses of Tor relays in the same /24 network around the time:</p>
              <ul>
                <li><a href="http://metrics.torproject.org/exonerator.html?ip=62.138.7.171&timestamp=2018-09-20&lang=en">62.138.7.171</a></li>
              </ul>
            </div><!-- panel-body -->

The underlying issue is that we have an Apache running on the metrics host that listens on 443 and rewrites to 8080. In our servlet, we don't even learn that the request came in via https.

I don't really have an elegant solution. The best thing I can come up with is that we pretend that we're living in an HTTPS world now and simply rewrite http to https. And for local testing environments we provide a simple configuration option that turns off this internal rewriting.

Changing to needs_information to collect feedback on this plan. If I don't hear otherwise, I'll hack something next week. Unless somebody else wants to do it, in which case, please just grab the ticket!

Trac:
Status: new to needs_information
Cc: N/A to metrics-team

For local testing environments you need HTTPS anyway or half the browser features are disabled and none of your JavaScript runs. Wherever we are using absolute links, we can set these to https://metrics.torproject.org/. Ideally though most of our links would be relative links.

Usually web applications have a "base URL" setting that can be overridden. Can we set this as a property with the default being https://metrics.torproject.org/?

Trac:
Status: needs_information to new

Replying to irl:

For local testing environments you need HTTPS anyway or half the browser features are disabled and none of your JavaScript runs.

Hmm, I usually don't have my local testing instance on HTTPS. But I also don't rely much on JavaScript running or not.

Wherever we are using absolute links, we can set these to https://metrics.torproject.org/.

Without looking at the code right now, I think that we might have to extend that to https://metrics.torproject.org/exonerator.html. But I'm not sure.

Ideally though most of our links would be relative links.

True, in theory. Not sure if there are any practical reasons against doing that.

Usually web applications have a "base URL" setting that can be overridden. Can we set this as a property with the default being https://metrics.torproject.org/?

Sounds fine to me!

Not grabbing just yet. If this is still owned by metrics-team in a couple of days I might grab it.

Alright, I wrote something with a base URL and relative links. Please review ExoneraTor commit d165e4a in my task-27925 branch.

If this gets merged and released, we'll only have to update the ExoneraTor version in metrics-web and include a base URL in its deployment descriptor as follows:

diff --git a/src/main/resources/web.xml b/src/main/resources/web.xml
index 08a3bcd0..66cc3daa 100644
--- a/src/main/resources/web.xml
+++ b/src/main/resources/web.xml
@@ -482,6 +482,10 @@
       /srv/metrics.torproject.org/metrics/website/rserve/graphs/
     </param-value>
   </context-param>
+  <context-param>
+    <param-name>baseUrl</param-name>
+    <param-value>https://metrics.torproject.org</param-value>
+  </context-param>
 
   <listener>
     <listener-class>

Trac:
Status: new to needs_review

This all looks good to me, and I think is the most correct way to fix it. (:

Trac:
Status: needs_review to merge_ready

Good to hear! (I accidentally already merged that patch when updating to the latest metrics-base; oops.) Next steps are to put out an ExoneraTor release and update metrics-web as mentioned above.

All done. Closing.

Trac:
Status: merge_ready to closed
Resolution: N/A to fixed

closed