Opened 3 years ago

Closed 3 months ago

#22062 closed defect (fixed)

Bad requests do not add the Access-Control-Allow-Origin header

Reported by: cypherpunks Owned by: metrics-team
Priority: Medium Milestone:
Component: Metrics/Onionoo Version:
Severity: Normal Keywords: metrics-2018
Cc: Actual Points:
Parent ID: Points:
Reviewer: irl Sponsor:


I've found this issue by submitting invalid requests through Atlas. For example, shows in the Firefox Developer Tools Console tab the following message.

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at (Reason: CORS header 'Access-Control-Allow-Origin' missing).

Looking at the Onionoo code, the culprit seems to be the doGet function and more specifically which sets several headers but only when the request has been processed successfully. For invalid requests it calls sendError. According to the Java API sendError

Sends an error response to the client using the specified status code and clears the buffer. The server will preserve cookies and may clear or update any headers needed to serve the error page as a valid response. If an error-page declaration has been made for the web application corresponding to the status code passed in, it will be served back the error page

I'm not sure which headers sendError clears or updates but I think the setHeader calls can safely be moved to the top of the doGet function to ensure these headers are also included in bad request responses. Unfortunately i have no setup to test this assumption/solution.

Child Tickets

Change History (6)

comment:1 Changed 3 years ago by karsten

Before I try out whether this works and what it might break in unexpected ways: what's the use case for including these headers in responses to bad requests?

comment:2 in reply to:  1 Changed 3 years ago by cypherpunks

Replying to karsten:

Before I try out whether this works and what it might break in unexpected ways: what's the use case for including these headers in responses to bad requests?

I can't speak about the Cache-Control header but including the Access-Control-Allow-Origin header for responses to bad requests would allow browsers to fully load the response and prevent CORS errors from showing up in the console.

comment:3 Changed 2 years ago by karsten

Keywords: metrics-2018 added

comment:4 Changed 2 years ago by irl

This would be required for #8667.

comment:5 Changed 3 months ago by karsten

Reviewer: irl
Status: newneeds_review

Okay, I spent way more time on this than I should have. Here's what I found out:

We can indeed set the Access-Control-Allow-Origin * header at the top of doGet:

diff --git a/src/main/java/org/torproject/metrics/onionoo/server/ b/src/main/java/org/torproject/metrics/onionoo/server/
index 0fee44b..b46c465 100644
--- a/src/main/java/org/torproject/metrics/onionoo/server/
+++ b/src/main/java/org/torproject/metrics/onionoo/server/
@@ -96,6 +96,8 @@ public class ResourceServlet extends HttpServlet {
       HttpServletResponseWrapper response, long receivedRequestMillis)
       throws IOException {
+    response.setHeader("Access-Control-Allow-Origin", "*");
     if (this.maintenanceMode) {
@@ -408,7 +410,6 @@ public class ResourceServlet extends HttpServlet {
         ((CACHE_MAX_TIME - indexAgeMillis)
-    response.setHeader("Access-Control-Allow-Origin", "*");
     response.setHeader("Cache-Control", "public, max-age="

The effect is that all requests being handled by this servlet class will have that header, regardless of response code. This includes URLs starting with /summary, /details, /bandwidth, /weights, /clients, and /uptime. It does not, however, apply for other resources including typos of those URLs.

The same does not work for cache headers. These are overwritten when we call sendError(), and I don't see an easy way to prevent that from happening. Maybe a filter or another handler or whatever Jetty has for such cases. Unfortunately, I don't know Jetty well enough to configure this, and I lack the time to find out.

My suggestion would be that we move the Access-Control-Allow-Origin * header at the top of doGet as seen in the diff above and leave caching unchanged. How does this sound?

comment:6 Changed 3 months ago by karsten

Resolution: fixed
Status: needs_reviewclosed

I'm about to put out a new Onionoo release, and I'm going to include this really minor change there. We can look into headers and caching in #32065. Merging and closing. Thanks!

Note: See TracTickets for help on using tickets.