Default to HEAD requests when the Companion looks to get meta information about a URL

[zackbloom]

At present, the URL loading component of the Companion makes two GET requests to the target URL. The first is only used to gather meta information (the files Content-Length and Content-Type), and then is aborted. The second downloads the file. Of course, depending on how much of the body has already been sent, and how well the delivering server understands aborts, this may result in a significant amount of unnecessary body data also being sent in that first meta request.

HTTP has a way to gather meta information about a file, the HEAD HTTP method. It is specified that a response from HEAD should include the Content-Length that will be returned from a GET request, making this a suitable option assuming the server in question implements HEAD correctly.

This change optimistically attempts a HEAD request. If it fails, or it doesn't return a meaningful Content-Length, it fallsback to the GET request currently used.

Compatibility

I believe this should not change the behavior of the module significantly unless:

• The URL being fetched implements HEAD in a truly standards-incompliant way which returns a value, but not the right value
• The HEAD request takes a very long time to return it's non-result, slowing down the overall download

Value

Conversely, I believe that this will slightly improve performance, and potentially significantly decrease server overhead, for URLs which implement standards correctly. Given that I imagine most URLs point to well-behaved instances of popular web servers, I think that probably amounts to 99%+ of the requests being made. Given we still will always fallback to a GET request on failure, I think the percentage of URLs this will cause issues with is quite small.

Note

I also included a tiny additional change, adding CHUNK_SIZE as an option which can be specified using env vars. Apologies if it can't be merged in this PR.

[mifi]

Thanks for your PR! I agree it make sense to try HEAD first and then retry with GET. I took the liberty of updating doc and rewriting the logic a bit so it reads more easily (imo). Maybe @Murderlon wants to do a review. I did some testing and it still works with S3 signed URLs

[mifi]

The main disadvantage I see is that it will slightly slow down requests to signed S3 urls, as it has to do a HEAD first and then GET, and a lot of companies use S3 to store files.

[zackbloom]

I think you're rewrite is fantastic @mifi! Phenomenal job making the code more clear.

A simple partial solution to the issue you describe is to look if the hostname ends with (^|\.)s3(.|\-)([a-z\-]+\.)?amazonaws.com. That should match:

s3.amazonaws.com
s3-us-west-2.amazonaws.com
s3.us-west-2.amazonaws.com

[mifi]

Interesting idea! Do you have any source for this? If we implement such a check I would prefer if we can link to an official source showing the list of possible hostnames for S3

[mifi]

I will merge this, thanks! We can optimize in the future if needed, but I don't think it should be a big problem for now.