From 33131cbcdc77ec1ce68a6497bc0b2d74e7a3117c Mon Sep 17 00:00:00 2001 From: Ryan Tomayko Date: Sun, 13 Mar 2011 23:19:02 -0700 Subject: [PATCH 01/18] for when query strings aren't enough, a new URL format: '//' Some caching proxies *cough* cloudfront custom origin *cough* do not include any query string information when making requests on the origin server. This commit adds support for a second URL format: http://camo.example.org// is the normal HMAC digest; is the aggressively URL escaped image URL. When this URL format is detected, the query string is ignored. This should be 100% backward compatible with existing URLs. The testsuite now runs the same set of tests on both the query string based URLs as well as the new path based URLs. --- server.coffee | 11 +++++++---- test/proxy_test.rb | 38 +++++++++++++++++++++++++++----------- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/server.coffee b/server.coffee index 8870bcb..4289d7a 100644 --- a/server.coffee +++ b/server.coffee @@ -58,16 +58,19 @@ server = Http.createServer (req, resp) -> delete(req.headers.cookie) log(req.headers) - query_digest = url.pathname.replace(/^\//, '') - query_params = QueryString.parse(url.query) + [query_digest, dest_url...] = url.pathname.replace(/^\//, '').split("/") + if dest_url.length > 0 + dest_url = unescape(dest_url.join("/")) + else + dest_url = QueryString.parse(url.query).url if url.pathname? hmac = Crypto.createHmac("sha1", shared_key) - hmac.update(query_params.url) + hmac.update(dest_url) hmac_digest = hmac.digest('hex') if hmac_digest == query_digest - url = Url.parse query_params.url + url = Url.parse dest_url if url.host? && !url.host.match(RESTRICTED_IPS) if url.host.match(EXCLUDED_HOSTS) diff --git a/test/proxy_test.rb b/test/proxy_test.rb index 4c49308..172a2d1 100644 --- a/test/proxy_test.rb +++ b/test/proxy_test.rb @@ -7,22 +7,12 @@ require 'test/unit' -class CamoProxyTest < Test::Unit::TestCase +module CamoProxyTests def config { 'key' => ENV['CAMO_KEY'] || "0x24FEEDFACEDEADBEEFCAFE", 'host' => ENV['CAMO_HOST'] || "http://localhost:8081" } end - def request(image_url) - hexdigest = OpenSSL::HMAC.hexdigest( - OpenSSL::Digest::Digest.new('sha1'), config['key'], image_url) - - uri = Addressable::URI.parse("#{config['host']}/#{hexdigest}") - uri.query_values = { 'url' => image_url, 'repo' => '', 'path' => '' } - - RestClient.get(uri.to_s) - end - def test_proxy_valid_image_url response = request('http://media.ebaumsworld.com/picture/Mincemeat/Pimp.jpg') assert_equal(200, response.code) @@ -95,3 +85,29 @@ def test_404s_on_environmental_excludes end end end + +class CamoProxyQueryStringTest < Test::Unit::TestCase + include CamoProxyTests + + def request(image_url) + hexdigest = OpenSSL::HMAC.hexdigest( + OpenSSL::Digest::Digest.new('sha1'), config['key'], image_url) + + uri = Addressable::URI.parse("#{config['host']}/#{hexdigest}") + uri.query_values = { 'url' => image_url, 'repo' => '', 'path' => '' } + + RestClient.get(uri.to_s) + end +end + +class CamoProxyPathTest < Test::Unit::TestCase + include CamoProxyTests + + def request(image_url) + hexdigest = OpenSSL::HMAC.hexdigest( + OpenSSL::Digest::Digest.new('sha1'), config['key'], image_url) + encoded_image_url = URI.escape(image_url, Regexp.new("[^#{URI::PATTERN::UNRESERVED}]")) + uri = "#{config['host']}/#{hexdigest}/#{encoded_image_url}" + RestClient.get(uri) + end +end From df971367b03d3ba7cd65a1fe65e71bbd3f97178c Mon Sep 17 00:00:00 2001 From: Ryan Tomayko Date: Sun, 13 Mar 2011 23:23:16 -0700 Subject: [PATCH 02/18] log the original url, url type, and digest in addition to headers --- server.coffee | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/server.coffee b/server.coffee index 4289d7a..8916135 100644 --- a/server.coffee +++ b/server.coffee @@ -56,14 +56,23 @@ server = Http.createServer (req, resp) -> 'x-content-type-options' : 'nosniff' delete(req.headers.cookie) - log(req.headers) [query_digest, dest_url...] = url.pathname.replace(/^\//, '').split("/") if dest_url.length > 0 + url_type = 'path' dest_url = unescape(dest_url.join("/")) else + url_type = 'query' dest_url = QueryString.parse(url.query).url + log({ + type: url_type + url: req.url + headers: req.headers + dest: dest_url + digest: query_digest + }) + if url.pathname? hmac = Crypto.createHmac("sha1", shared_key) hmac.update(dest_url) From 2a3d786bf59f40c95be2fa16bd04de78b93538d4 Mon Sep 17 00:00:00 2001 From: Ryan Tomayko Date: Sun, 13 Mar 2011 23:34:03 -0700 Subject: [PATCH 03/18] URL format docs in the README --- README.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 21cd9de..4e2cc0a 100644 --- a/README.md +++ b/README.md @@ -11,14 +11,26 @@ Using a shared key, proxy URLs are encrypted with [hmac](http://en.wikipedia.org Features -------- -* Proxy remote images with a content-type of images/* -* Proxy images < 5 MB +* Proxy remote images with a content-type of `image/*` +* Proxy images under 5 MB * Proxy google charts * 404s for anything other than a 200 or 304 HTTP response * Disallows proxying to private IP ranges At GitHub we render markdown and replace all of the `src` attributes on the `img` tags with the appropriate URL to hit the proxies. There's example code for creating URLs in [the tests](https://github.com/atmos/camo/blob/master/test/proxy_test.rb). +## URL Formats + +Camo supports two distinct URL formats: + + http://example.org/?url= + http://example.org// + +The `` is a 40 character hex encoded HMAC digest generated with a shared +secret key and the unescaped `` value. The `` is the absolute +URL locating an image. In either format, the `` should be URL escaped +aggressively to ensure the original value isn't mangled in transit. + ## Testing Functionality ### Start the server From 3ccfb0e1f6768f6582e5d587908688e72d9b1b81 Mon Sep 17 00:00:00 2001 From: Ryan Tomayko Date: Mon, 14 Mar 2011 00:22:15 -0700 Subject: [PATCH 04/18] server.js update --- server.js | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/server.js b/server.js index c97a169..82b6b32 100644 --- a/server.js +++ b/server.js @@ -1,5 +1,6 @@ (function() { var Crypto, EXCLUDED_HOSTS, Fs, Http, QueryString, RESTRICTED_IPS, Url, current_connections, excluded, finish, four_oh_four, log, logging_enabled, port, server, shared_key, started_at, total_connections, version; + var __slice = Array.prototype.slice; Fs = require('fs'); Url = require('url'); Http = require('http'); @@ -35,7 +36,7 @@ return resp.end(str); }; server = Http.createServer(function(req, resp) { - var hmac, hmac_digest, query_digest, query_params, query_path, src, srcReq, transferred_headers, url, _base; + var dest_url, hmac, hmac_digest, query_digest, query_path, src, srcReq, transferred_headers, url, url_type, _base, _ref; if (req.method !== 'GET' || req.url === '/') { resp.writeHead(200); return resp.end('hwhat'); @@ -57,15 +58,27 @@ 'x-content-type-options': 'nosniff' }; delete req.headers.cookie; - log(req.headers); - query_digest = url.pathname.replace(/^\//, ''); - query_params = QueryString.parse(url.query); + _ref = url.pathname.replace(/^\//, '').split("/"), query_digest = _ref[0], dest_url = 2 <= _ref.length ? __slice.call(_ref, 1) : []; + if (dest_url.length > 0) { + url_type = 'path'; + dest_url = unescape(dest_url.join("/")); + } else { + url_type = 'query'; + dest_url = QueryString.parse(url.query).url; + } + log({ + type: url_type, + url: req.url, + headers: req.headers, + dest: dest_url, + digest: query_digest + }); if (url.pathname != null) { hmac = Crypto.createHmac("sha1", shared_key); - hmac.update(query_params.url); + hmac.update(dest_url); hmac_digest = hmac.digest('hex'); if (hmac_digest === query_digest) { - url = Url.parse(query_params.url); + url = Url.parse(dest_url); if ((url.host != null) && !url.host.match(RESTRICTED_IPS)) { if (url.host.match(EXCLUDED_HOSTS)) { return four_oh_four(resp, "Hitting excluded hostnames"); @@ -111,7 +124,6 @@ return srcResp.on('data', function(chunk) { return resp.write(chunk); }); - break; case 304: return resp.writeHead(srcResp.statusCode, newHeaders); default: From 4cbaa3cc391b09eb168d1198831ecb610439822a Mon Sep 17 00:00:00 2001 From: Ryan Tomayko Date: Mon, 14 Mar 2011 01:58:37 -0700 Subject: [PATCH 05/18] use more aggressive hex encoding instead of url encoding for path format Rolling with this for now due to some insane URL decoding in older versions of node (<= v0.2.4). Ostensibly, old node versions decode the piss out of anything in the path part of the URL, include %2F and also squash // characters together. The result is that you cannot cleanly separate a URL encoded value in the path. With this new scheme, every byte of the image URL is encoded into a 2 char hex value. I actually kind of like it for its similarity to the digest hex encoding. --- README.md | 8 +++++--- server.coffee | 14 +++++++++++--- server.js | 21 +++++++++++++++------ test/proxy_test.rb | 6 +++++- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 4e2cc0a..2bc0a79 100644 --- a/README.md +++ b/README.md @@ -27,9 +27,11 @@ Camo supports two distinct URL formats: http://example.org// The `` is a 40 character hex encoded HMAC digest generated with a shared -secret key and the unescaped `` value. The `` is the absolute -URL locating an image. In either format, the `` should be URL escaped -aggressively to ensure the original value isn't mangled in transit. +secret key and the unescaped `` value. The `` is the +absolute URL locating an image. In the first format, the `` should be +URL escaped aggressively to ensure the original value isn't mangled in transit. +In the second format, each byte of the `` should be hex encoded such +that the resulting value includes only characters `[0-9a-f]`. ## Testing Functionality diff --git a/server.coffee b/server.coffee index 8916135..d625821 100644 --- a/server.coffee +++ b/server.coffee @@ -33,6 +33,14 @@ finish = (resp, str) -> current_connections = 0 if current_connections < 1 resp.end str +# decode a string of two char hex digits +hexdec = (str) -> + if str and str.length > 0 and str.length % 2 == 0 and not str.match(/[^0-9a-f]/) + buf = new Buffer(str.length / 2) + for i in [0...str.length] by 2 + buf[i/2] = parseInt(str[i..i+1], 16) + buf.toString() + server = Http.createServer (req, resp) -> if req.method != 'GET' || req.url == '/' resp.writeHead 200 @@ -57,10 +65,10 @@ server = Http.createServer (req, resp) -> delete(req.headers.cookie) - [query_digest, dest_url...] = url.pathname.replace(/^\//, '').split("/") - if dest_url.length > 0 + [query_digest, encoded_url] = url.pathname.replace(/^\//, '').split("/", 2) + if encoded_url = hexdec(encoded_url) url_type = 'path' - dest_url = unescape(dest_url.join("/")) + dest_url = encoded_url else url_type = 'query' dest_url = QueryString.parse(url.query).url diff --git a/server.js b/server.js index 82b6b32..82bf60f 100644 --- a/server.js +++ b/server.js @@ -1,6 +1,5 @@ (function() { - var Crypto, EXCLUDED_HOSTS, Fs, Http, QueryString, RESTRICTED_IPS, Url, current_connections, excluded, finish, four_oh_four, log, logging_enabled, port, server, shared_key, started_at, total_connections, version; - var __slice = Array.prototype.slice; + var Crypto, EXCLUDED_HOSTS, Fs, Http, QueryString, RESTRICTED_IPS, Url, current_connections, excluded, finish, four_oh_four, hexdec, log, logging_enabled, port, server, shared_key, started_at, total_connections, version; Fs = require('fs'); Url = require('url'); Http = require('http'); @@ -35,8 +34,18 @@ } return resp.end(str); }; + hexdec = function(str) { + var buf, i, _ref; + if (str && str.length > 0 && str.length % 2 === 0 && !str.match(/[^0-9a-f]/)) { + buf = new Buffer(str.length / 2); + for (i = 0, _ref = str.length; (0 <= _ref ? i < _ref : i > _ref); i += 2) { + buf[i / 2] = parseInt(str.slice(i, (i + 1 + 1) || 9e9), 16); + } + return buf.toString(); + } + }; server = Http.createServer(function(req, resp) { - var dest_url, hmac, hmac_digest, query_digest, query_path, src, srcReq, transferred_headers, url, url_type, _base, _ref; + var dest_url, encoded_url, hmac, hmac_digest, query_digest, query_path, src, srcReq, transferred_headers, url, url_type, _base, _ref; if (req.method !== 'GET' || req.url === '/') { resp.writeHead(200); return resp.end('hwhat'); @@ -58,10 +67,10 @@ 'x-content-type-options': 'nosniff' }; delete req.headers.cookie; - _ref = url.pathname.replace(/^\//, '').split("/"), query_digest = _ref[0], dest_url = 2 <= _ref.length ? __slice.call(_ref, 1) : []; - if (dest_url.length > 0) { + _ref = url.pathname.replace(/^\//, '').split("/", 2), query_digest = _ref[0], encoded_url = _ref[1]; + if (encoded_url = hexdec(encoded_url)) { url_type = 'path'; - dest_url = unescape(dest_url.join("/")); + dest_url = encoded_url; } else { url_type = 'query'; dest_url = QueryString.parse(url.query).url; diff --git a/test/proxy_test.rb b/test/proxy_test.rb index 172a2d1..34373db 100644 --- a/test/proxy_test.rb +++ b/test/proxy_test.rb @@ -103,10 +103,14 @@ def request(image_url) class CamoProxyPathTest < Test::Unit::TestCase include CamoProxyTests + def hexenc(image_url) + image_url.to_enum(:each_byte).map { |byte| "%02x" % byte }.join + end + def request(image_url) hexdigest = OpenSSL::HMAC.hexdigest( OpenSSL::Digest::Digest.new('sha1'), config['key'], image_url) - encoded_image_url = URI.escape(image_url, Regexp.new("[^#{URI::PATTERN::UNRESERVED}]")) + encoded_image_url = hexenc(image_url) uri = "#{config['host']}/#{hexdigest}/#{encoded_image_url}" RestClient.get(uri) end From 56738e60d4412aac11daf250fa17aa6b29f094da Mon Sep 17 00:00:00 2001 From: Ryan Tomayko Date: Mon, 14 Mar 2011 02:03:53 -0700 Subject: [PATCH 06/18] add a build task --- Rakefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index f236fee..47f4242 100644 --- a/Rakefile +++ b/Rakefile @@ -1,10 +1,15 @@ +file 'server.js' => 'server.coffee' do + sh "coffee -c -o . server.coffee" +end +task :build => 'server.js' + namespace :test do desc "Run the tests against localhost" task :check do |t| system("ruby test/proxy_test.rb") end end -task :default => "test:check" +task :default => [:build, "test:check"] Dir["tasks/*.rake"].each do |f| load f From 90d0e3fc19a0e29413154fd3eea387cf10bfffa8 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Fri, 6 May 2011 12:10:25 -0700 Subject: [PATCH 07/18] showoff bad regex test --- test/proxy_test.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/proxy_test.rb b/test/proxy_test.rb index 34373db..3f1bc15 100644 --- a/test/proxy_test.rb +++ b/test/proxy_test.rb @@ -18,6 +18,11 @@ def test_proxy_valid_image_url assert_equal(200, response.code) end + def test_proxy_valid_image_url_with_crazy_subdomain + response = request('http://27.media.tumblr.com/tumblr_lkp6rdDfRi1qce6mto1_500.jpg') + assert_equal(200, response.code) + end + def test_proxy_valid_google_chart_url response = request('http://chart.apis.google.com/chart?chs=920x200&chxl=0:%7C2010-08-13%7C2010-09-12%7C2010-10-12%7C2010-11-11%7C1:%7C0%7C0%7C0%7C0%7C0%7C0&chm=B,EBF5FB,0,0,0&chco=008Cd6&chls=3,1,0&chg=8.3,20,1,4&chd=s:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA&chxt=x,y&cht=lc') assert_equal(200, response.code) From b5cf4fed206ded1039b36a65330fb132fa7f1abb Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Fri, 6 May 2011 13:38:09 -0700 Subject: [PATCH 08/18] camo fixup for valid domains starting with numbers --- server.coffee | 2 +- server.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server.coffee b/server.coffee index d625821..81beb6d 100644 --- a/server.coffee +++ b/server.coffee @@ -17,7 +17,7 @@ log = (msg) -> console.log("--------------------------------------------") EXCLUDED_HOSTS = new RegExp(excluded.replace(".", "\\.").replace("*", "\\.*")) -RESTRICTED_IPS = /^(10\.)|(127\.)|(169\.254)|(192\.168)|(172\.(1[6-9])|(2[0-9])|(3[0-1]))/ +RESTRICTED_IPS = /^((10\.)|(127\.)|(169\.254)|(192\.168)|(172\.((1[6-9])|(2[0-9])|(3[0-1]))))/ total_connections = 0 current_connections = 0 diff --git a/server.js b/server.js index 82bf60f..94218d7 100644 --- a/server.js +++ b/server.js @@ -18,7 +18,7 @@ } }; EXCLUDED_HOSTS = new RegExp(excluded.replace(".", "\\.").replace("*", "\\.*")); - RESTRICTED_IPS = /^(10\.)|(127\.)|(169\.254)|(192\.168)|(172\.(1[6-9])|(2[0-9])|(3[0-1]))/; + RESTRICTED_IPS = /^((10\.)|(127\.)|(169\.254)|(192\.168)|(172\.((1[6-9])|(2[0-9])|(3[0-1]))))/; total_connections = 0; current_connections = 0; started_at = new Date; @@ -38,7 +38,7 @@ var buf, i, _ref; if (str && str.length > 0 && str.length % 2 === 0 && !str.match(/[^0-9a-f]/)) { buf = new Buffer(str.length / 2); - for (i = 0, _ref = str.length; (0 <= _ref ? i < _ref : i > _ref); i += 2) { + for (i = 0, _ref = str.length; 0 <= _ref ? i < _ref : i > _ref; i += 2) { buf[i / 2] = parseInt(str.slice(i, (i + 1 + 1) || 9e9), 16); } return buf.toString(); From 93feae8706ab6646cb2e8f30fe34a99f30bcd77d Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Thu, 30 Jun 2011 15:29:28 -0700 Subject: [PATCH 09/18] don't finish off response errors --- server.coffee | 4 ++-- server.js | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/server.coffee b/server.coffee index 81beb6d..1685968 100644 --- a/server.coffee +++ b/server.coffee @@ -126,8 +126,8 @@ server = Http.createServer (req, resp) -> srcResp.on 'end', -> finish resp - srcResp.on 'error', -> - finish resp + # srcResp.on 'error', -> + # finish resp switch srcResp.statusCode when 200 diff --git a/server.js b/server.js index 94218d7..d1b79a0 100644 --- a/server.js +++ b/server.js @@ -35,10 +35,10 @@ return resp.end(str); }; hexdec = function(str) { - var buf, i, _ref; + var buf, i, _ref, _step; if (str && str.length > 0 && str.length % 2 === 0 && !str.match(/[^0-9a-f]/)) { buf = new Buffer(str.length / 2); - for (i = 0, _ref = str.length; 0 <= _ref ? i < _ref : i > _ref; i += 2) { + for (i = 0, _ref = str.length, _step = 2; 0 <= _ref ? i < _ref : i > _ref; i += _step) { buf[i / 2] = parseInt(str.slice(i, (i + 1 + 1) || 9e9), 16); } return buf.toString(); @@ -120,9 +120,6 @@ srcResp.on('end', function() { return finish(resp); }); - srcResp.on('error', function() { - return finish(resp); - }); switch (srcResp.statusCode) { case 200: if (newHeaders['content-type'] && newHeaders['content-type'].slice(0, 5) !== 'image') { From 31d890daaf7b9da3bb112e28c8cc9a4b30915f77 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Thu, 30 Jun 2011 15:37:26 -0700 Subject: [PATCH 10/18] Revert "don't finish off response errors" This reverts commit 93feae8706ab6646cb2e8f30fe34a99f30bcd77d. --- server.coffee | 4 ++-- server.js | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/server.coffee b/server.coffee index 1685968..81beb6d 100644 --- a/server.coffee +++ b/server.coffee @@ -126,8 +126,8 @@ server = Http.createServer (req, resp) -> srcResp.on 'end', -> finish resp - # srcResp.on 'error', -> - # finish resp + srcResp.on 'error', -> + finish resp switch srcResp.statusCode when 200 diff --git a/server.js b/server.js index d1b79a0..94218d7 100644 --- a/server.js +++ b/server.js @@ -35,10 +35,10 @@ return resp.end(str); }; hexdec = function(str) { - var buf, i, _ref, _step; + var buf, i, _ref; if (str && str.length > 0 && str.length % 2 === 0 && !str.match(/[^0-9a-f]/)) { buf = new Buffer(str.length / 2); - for (i = 0, _ref = str.length, _step = 2; 0 <= _ref ? i < _ref : i > _ref; i += _step) { + for (i = 0, _ref = str.length; 0 <= _ref ? i < _ref : i > _ref; i += 2) { buf[i / 2] = parseInt(str.slice(i, (i + 1 + 1) || 9e9), 16); } return buf.toString(); @@ -120,6 +120,9 @@ srcResp.on('end', function() { return finish(resp); }); + srcResp.on('error', function() { + return finish(resp); + }); switch (srcResp.statusCode) { case 200: if (newHeaders['content-type'] && newHeaders['content-type'].slice(0, 5) !== 'image') { From e2bb1ea938b3d4a21a045a60f7092ff36459ca42 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Thu, 30 Jun 2011 15:38:13 -0700 Subject: [PATCH 11/18] try one of the other error blocks --- server.coffee | 4 ++-- server.js | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/server.coffee b/server.coffee index 81beb6d..08df49b 100644 --- a/server.coffee +++ b/server.coffee @@ -146,8 +146,8 @@ server = Http.createServer (req, resp) -> else four_oh_four(resp, "Responded with #{srcResp.statusCode}:#{srcResp.headers}") - srcReq.on 'error', -> - finish resp + # srcReq.on 'error', -> + # finish resp srcReq.end() diff --git a/server.js b/server.js index 94218d7..a6a8f62 100644 --- a/server.js +++ b/server.js @@ -35,10 +35,10 @@ return resp.end(str); }; hexdec = function(str) { - var buf, i, _ref; + var buf, i, _ref, _step; if (str && str.length > 0 && str.length % 2 === 0 && !str.match(/[^0-9a-f]/)) { buf = new Buffer(str.length / 2); - for (i = 0, _ref = str.length; 0 <= _ref ? i < _ref : i > _ref; i += 2) { + for (i = 0, _ref = str.length, _step = 2; 0 <= _ref ? i < _ref : i > _ref; i += _step) { buf[i / 2] = parseInt(str.slice(i, (i + 1 + 1) || 9e9), 16); } return buf.toString(); @@ -140,9 +140,6 @@ } } }); - srcReq.on('error', function() { - return finish(resp); - }); return srcReq.end(); } else { return four_oh_four(resp, "No host found " + url.host); From 63ba17df2a3c2dd064fff50a8af436780e814f3c Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Thu, 30 Jun 2011 15:40:59 -0700 Subject: [PATCH 12/18] Revert "try one of the other error blocks" This reverts commit e2bb1ea938b3d4a21a045a60f7092ff36459ca42. --- server.coffee | 4 ++-- server.js | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/server.coffee b/server.coffee index 08df49b..81beb6d 100644 --- a/server.coffee +++ b/server.coffee @@ -146,8 +146,8 @@ server = Http.createServer (req, resp) -> else four_oh_four(resp, "Responded with #{srcResp.statusCode}:#{srcResp.headers}") - # srcReq.on 'error', -> - # finish resp + srcReq.on 'error', -> + finish resp srcReq.end() diff --git a/server.js b/server.js index a6a8f62..94218d7 100644 --- a/server.js +++ b/server.js @@ -35,10 +35,10 @@ return resp.end(str); }; hexdec = function(str) { - var buf, i, _ref, _step; + var buf, i, _ref; if (str && str.length > 0 && str.length % 2 === 0 && !str.match(/[^0-9a-f]/)) { buf = new Buffer(str.length / 2); - for (i = 0, _ref = str.length, _step = 2; 0 <= _ref ? i < _ref : i > _ref; i += _step) { + for (i = 0, _ref = str.length; 0 <= _ref ? i < _ref : i > _ref; i += 2) { buf[i / 2] = parseInt(str.slice(i, (i + 1 + 1) || 9e9), 16); } return buf.toString(); @@ -140,6 +140,9 @@ } } }); + srcReq.on('error', function() { + return finish(resp); + }); return srcReq.end(); } else { return four_oh_four(resp, "No host found " + url.host); From 335315141f818a642d83447e1313c12099d3c16c Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Thu, 30 Jun 2011 15:42:55 -0700 Subject: [PATCH 13/18] exception whack-a-mole --- server.coffee | 2 +- server.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server.coffee b/server.coffee index 81beb6d..3b5ad7f 100644 --- a/server.coffee +++ b/server.coffee @@ -31,7 +31,7 @@ four_oh_four = (resp, msg) -> finish = (resp, str) -> current_connections -= 1 current_connections = 0 if current_connections < 1 - resp.end str + resp.connection && resp.end str # decode a string of two char hex digits hexdec = (str) -> diff --git a/server.js b/server.js index 94218d7..510c97b 100644 --- a/server.js +++ b/server.js @@ -32,13 +32,13 @@ if (current_connections < 1) { current_connections = 0; } - return resp.end(str); + return resp.connection && resp.end(str); }; hexdec = function(str) { - var buf, i, _ref; + var buf, i, _ref, _step; if (str && str.length > 0 && str.length % 2 === 0 && !str.match(/[^0-9a-f]/)) { buf = new Buffer(str.length / 2); - for (i = 0, _ref = str.length; 0 <= _ref ? i < _ref : i > _ref; i += 2) { + for (i = 0, _ref = str.length, _step = 2; 0 <= _ref ? i < _ref : i > _ref; i += _step) { buf[i / 2] = parseInt(str.slice(i, (i + 1 + 1) || 9e9), 16); } return buf.toString(); From af3bff65a22f73ad8be41bc03ed1a234608d09fd Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Thu, 30 Jun 2011 16:14:49 -0700 Subject: [PATCH 14/18] export the Camo-Host header --- server.coffee | 2 ++ server.js | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/server.coffee b/server.coffee index 3b5ad7f..ddd65fe 100644 --- a/server.coffee +++ b/server.coffee @@ -8,6 +8,7 @@ port = parseInt process.env.PORT || 8081 version = "0.3.0" excluded = process.env.CAMO_HOST_EXCLUSIONS || '*.example.org' shared_key = process.env.CAMO_KEY || '0x24FEEDFACEDEADBEEFCAFE' +camo_hostname = process.env.CAMO_HOSTNAME || "unknown" logging_enabled = process.env.CAMO_LOGGING_ENABLED || "disabled" log = (msg) -> @@ -121,6 +122,7 @@ server = Http.createServer (req, resp) -> 'content-type' : srcResp.headers['content-type'] 'cache-control' : srcResp.headers['cache-control'] 'content-length' : content_length + 'Camo-Host' : camo_hostname 'X-Content-Type-Options' : 'nosniff' srcResp.on 'end', -> diff --git a/server.js b/server.js index 510c97b..7766e84 100644 --- a/server.js +++ b/server.js @@ -1,5 +1,5 @@ (function() { - var Crypto, EXCLUDED_HOSTS, Fs, Http, QueryString, RESTRICTED_IPS, Url, current_connections, excluded, finish, four_oh_four, hexdec, log, logging_enabled, port, server, shared_key, started_at, total_connections, version; + var Crypto, EXCLUDED_HOSTS, Fs, Http, QueryString, RESTRICTED_IPS, Url, camo_hostname, current_connections, excluded, finish, four_oh_four, hexdec, log, logging_enabled, port, server, shared_key, started_at, total_connections, version; Fs = require('fs'); Url = require('url'); Http = require('http'); @@ -9,6 +9,7 @@ version = "0.3.0"; excluded = process.env.CAMO_HOST_EXCLUSIONS || '*.example.org'; shared_key = process.env.CAMO_KEY || '0x24FEEDFACEDEADBEEFCAFE'; + camo_hostname = process.env.CAMO_HOSTNAME || "unknown"; logging_enabled = process.env.CAMO_LOGGING_ENABLED || "disabled"; log = function(msg) { if (logging_enabled !== "disabled") { @@ -115,6 +116,7 @@ 'content-type': srcResp.headers['content-type'], 'cache-control': srcResp.headers['cache-control'], 'content-length': content_length, + 'Camo-Host': camo_hostname, 'X-Content-Type-Options': 'nosniff' }; srcResp.on('end', function() { From 8a951f0ab8833d3c41c0f5a490a6efc4c34219ff Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Thu, 30 Jun 2011 17:41:38 -0700 Subject: [PATCH 15/18] update the version of camo that should be used --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 2bc0a79..ae54999 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,8 @@ We want to allow people to keep embedding images in comments/issues/READMEs/goog Using a shared key, proxy URLs are encrypted with [hmac](http://en.wikipedia.org/wiki/HMAC) so we can bust caches/ban/rate limit if needed. +Camo currently runs on node version 0.4.8 in production at GitHub. + Features -------- From 0edb6051ca476931331ef40326e735cfd6f95dfc Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Fri, 1 Jul 2011 10:17:44 -0700 Subject: [PATCH 16/18] try to log the urls that are making things explode --- server.coffee | 6 +++++- server.js | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/server.coffee b/server.coffee index ddd65fe..ca4c4d0 100644 --- a/server.coffee +++ b/server.coffee @@ -84,7 +84,11 @@ server = Http.createServer (req, resp) -> if url.pathname? hmac = Crypto.createHmac("sha1", shared_key) - hmac.update(dest_url) + try + hmac.update(dest_url) + catch error + console.log "Error on #{req.url} - #{dest_url}" + hmac_digest = hmac.digest('hex') if hmac_digest == query_digest diff --git a/server.js b/server.js index 7766e84..2685c6b 100644 --- a/server.js +++ b/server.js @@ -85,7 +85,11 @@ }); if (url.pathname != null) { hmac = Crypto.createHmac("sha1", shared_key); - hmac.update(dest_url); + try { + hmac.update(dest_url); + } catch (error) { + console.log("Error on " + req.url + " - " + dest_url); + } hmac_digest = hmac.digest('hex'); if (hmac_digest === query_digest) { url = Url.parse(dest_url); From 39f6078a850e05f277a16220ce6a218582ed8493 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Fri, 1 Jul 2011 10:59:02 -0700 Subject: [PATCH 17/18] some incoming data has & escaped as & in the query string --- server.coffee | 3 ++- server.js | 5 +++-- test/proxy_test.rb | 11 +++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/server.coffee b/server.coffee index ca4c4d0..2b8cd03 100644 --- a/server.coffee +++ b/server.coffee @@ -66,7 +66,8 @@ server = Http.createServer (req, resp) -> delete(req.headers.cookie) - [query_digest, encoded_url] = url.pathname.replace(/^\//, '').split("/", 2) + pathname = url.pathname.replace(/&/, '&') + [query_digest, encoded_url] = pathname.replace(/^\//, '').split("/", 2) if encoded_url = hexdec(encoded_url) url_type = 'path' dest_url = encoded_url diff --git a/server.js b/server.js index 2685c6b..5ca12b3 100644 --- a/server.js +++ b/server.js @@ -46,7 +46,7 @@ } }; server = Http.createServer(function(req, resp) { - var dest_url, encoded_url, hmac, hmac_digest, query_digest, query_path, src, srcReq, transferred_headers, url, url_type, _base, _ref; + var dest_url, encoded_url, hmac, hmac_digest, pathname, query_digest, query_path, src, srcReq, transferred_headers, url, url_type, _base, _ref; if (req.method !== 'GET' || req.url === '/') { resp.writeHead(200); return resp.end('hwhat'); @@ -68,7 +68,8 @@ 'x-content-type-options': 'nosniff' }; delete req.headers.cookie; - _ref = url.pathname.replace(/^\//, '').split("/", 2), query_digest = _ref[0], encoded_url = _ref[1]; + pathname = url.pathname.replace(/&/, '&'); + _ref = pathname.replace(/^\//, '').split("/", 2), query_digest = _ref[0], encoded_url = _ref[1]; if (encoded_url = hexdec(encoded_url)) { url_type = 'path'; dest_url = encoded_url; diff --git a/test/proxy_test.rb b/test/proxy_test.rb index 3f1bc15..a5aee17 100644 --- a/test/proxy_test.rb +++ b/test/proxy_test.rb @@ -28,6 +28,17 @@ def test_proxy_valid_google_chart_url assert_equal(200, response.code) end + def test_hmac_update_obscure_error + urls = [ + "http://share.kyleneath.com/captures/Testing_-_TestingAppDelegate.m-20110309-142723.png", + "http://cdn.shopify.com/s/files/1/0051/4802/products/featured_2.jpg?1287582804" + ] + urls.each do |url| + response = request(url) + assert_equal(200, response.code) + end + end + def test_404s_on_urls_without_an_http_host assert_raise RestClient::ResourceNotFound do request('/picture/Mincemeat/Pimp.jpg') From 4bc6d00e19ee4963533b3123b265383e52edc54d Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Fri, 1 Jul 2011 11:28:49 -0700 Subject: [PATCH 18/18] dest_url needs to be defined, it's undefined on poorly formatted image urls --- server.coffee | 10 +++------- server.js | 13 ++++--------- test/proxy_test.rb | 28 ++++++++++++---------------- 3 files changed, 19 insertions(+), 32 deletions(-) diff --git a/server.coffee b/server.coffee index 2b8cd03..7b903df 100644 --- a/server.coffee +++ b/server.coffee @@ -66,8 +66,7 @@ server = Http.createServer (req, resp) -> delete(req.headers.cookie) - pathname = url.pathname.replace(/&/, '&') - [query_digest, encoded_url] = pathname.replace(/^\//, '').split("/", 2) + [query_digest, encoded_url] = url.pathname.replace(/^\//, '').split("/", 2) if encoded_url = hexdec(encoded_url) url_type = 'path' dest_url = encoded_url @@ -83,12 +82,9 @@ server = Http.createServer (req, resp) -> digest: query_digest }) - if url.pathname? + if url.pathname? && dest_url hmac = Crypto.createHmac("sha1", shared_key) - try - hmac.update(dest_url) - catch error - console.log "Error on #{req.url} - #{dest_url}" + hmac.update(dest_url) hmac_digest = hmac.digest('hex') diff --git a/server.js b/server.js index 5ca12b3..46b7514 100644 --- a/server.js +++ b/server.js @@ -46,7 +46,7 @@ } }; server = Http.createServer(function(req, resp) { - var dest_url, encoded_url, hmac, hmac_digest, pathname, query_digest, query_path, src, srcReq, transferred_headers, url, url_type, _base, _ref; + var dest_url, encoded_url, hmac, hmac_digest, query_digest, query_path, src, srcReq, transferred_headers, url, url_type, _base, _ref; if (req.method !== 'GET' || req.url === '/') { resp.writeHead(200); return resp.end('hwhat'); @@ -68,8 +68,7 @@ 'x-content-type-options': 'nosniff' }; delete req.headers.cookie; - pathname = url.pathname.replace(/&/, '&'); - _ref = pathname.replace(/^\//, '').split("/", 2), query_digest = _ref[0], encoded_url = _ref[1]; + _ref = url.pathname.replace(/^\//, '').split("/", 2), query_digest = _ref[0], encoded_url = _ref[1]; if (encoded_url = hexdec(encoded_url)) { url_type = 'path'; dest_url = encoded_url; @@ -84,13 +83,9 @@ dest: dest_url, digest: query_digest }); - if (url.pathname != null) { + if ((url.pathname != null) && dest_url) { hmac = Crypto.createHmac("sha1", shared_key); - try { - hmac.update(dest_url); - } catch (error) { - console.log("Error on " + req.url + " - " + dest_url); - } + hmac.update(dest_url); hmac_digest = hmac.digest('hex'); if (hmac_digest === query_digest) { url = Url.parse(dest_url); diff --git a/test/proxy_test.rb b/test/proxy_test.rb index a5aee17..20cef00 100644 --- a/test/proxy_test.rb +++ b/test/proxy_test.rb @@ -28,17 +28,6 @@ def test_proxy_valid_google_chart_url assert_equal(200, response.code) end - def test_hmac_update_obscure_error - urls = [ - "http://share.kyleneath.com/captures/Testing_-_TestingAppDelegate.m-20110309-142723.png", - "http://cdn.shopify.com/s/files/1/0051/4802/products/featured_2.jpg?1287582804" - ] - urls.each do |url| - response = request(url) - assert_equal(200, response.code) - end - end - def test_404s_on_urls_without_an_http_host assert_raise RestClient::ResourceNotFound do request('/picture/Mincemeat/Pimp.jpg') @@ -105,14 +94,18 @@ def test_404s_on_environmental_excludes class CamoProxyQueryStringTest < Test::Unit::TestCase include CamoProxyTests - def request(image_url) + def request_uri(image_url) hexdigest = OpenSSL::HMAC.hexdigest( OpenSSL::Digest::Digest.new('sha1'), config['key'], image_url) uri = Addressable::URI.parse("#{config['host']}/#{hexdigest}") uri.query_values = { 'url' => image_url, 'repo' => '', 'path' => '' } - RestClient.get(uri.to_s) + uri.to_s + end + + def request(image_url) + RestClient.get(request_uri(image_url)) end end @@ -123,11 +116,14 @@ def hexenc(image_url) image_url.to_enum(:each_byte).map { |byte| "%02x" % byte }.join end - def request(image_url) + def request_uri(image_url) hexdigest = OpenSSL::HMAC.hexdigest( OpenSSL::Digest::Digest.new('sha1'), config['key'], image_url) encoded_image_url = hexenc(image_url) - uri = "#{config['host']}/#{hexdigest}/#{encoded_image_url}" - RestClient.get(uri) + "#{config['host']}/#{hexdigest}/#{encoded_image_url}" + end + + def request(image_url) + RestClient.get(request_uri(image_url)) end end