getodk / aggregate

ODK Aggregate is a Java server that stores, analyzes, and presents survey data collected using ODK Collect. Contribute and make the world a better place! ✨🗄✨
https://docs.opendatakit.org/aggregate-intro/
Other
74 stars 227 forks source link

Aggregate should not add Location header to HTTP 204 responses #409

Closed ggalmazor closed 5 years ago

ggalmazor commented 5 years ago

Problem description

Aggregate adds Location header to HTTP 204 responses, which is not a standard practice and can confuse clients into thinking that they need to do a redirect.

Steps to reproduce the problem

Here's one method to trigger this:

Expected behavior

No Location header should be present in 204 responses.

Other information

N/A

yanokwa commented 5 years ago

There might be a good reason for this. I vaguely remember something about supporting coffee shop Wi-Fi login pages? I would dig in into Aggregate's history before making this.

ggalmazor commented 5 years ago

I've learned that 201 responses can use the optional Location header to show the URI of a newly created server-side object, which it's what's happening here. If no Location header is added to the response, the effective request URI should be considered as the object's URL

Although this behavior is conforming to the HTTP standard, Aggregate already has an API with an endpoint to download objects which doesn't match the "effective request URI" and makes adding a Location header the only reasonable option.

In any case, we could make this issue about veryfing that we put the correct API endpoint in the Location header, but I don't see much value on doing that at this point. What do you think?