From ca09a61b5260f8d4367ba4ab9b85e163ffaea13c Mon Sep 17 00:00:00 2001
From: Marcin Kulik <m@ku1ik.com>
Date: Mon, 23 Mar 2015 17:40:11 +0100
Subject: [PATCH] Handle invalid asciicast upload

---
 app/controllers/api/asciicasts_controller.rb |  2 ++
 app/models/asciicast_params.rb               | 10 ++++++++--
 spec/api/asciicast_create_spec.rb            | 21 ++++++++++++++++++--
 spec/fixtures/1/invalid.json                 | 16 +++++++++++++++
 4 files changed, 45 insertions(+), 4 deletions(-)
 create mode 100644 spec/fixtures/1/invalid.json

diff --git a/app/controllers/api/asciicasts_controller.rb b/app/controllers/api/asciicasts_controller.rb
index 2ac601b..df50933 100644
--- a/app/controllers/api/asciicasts_controller.rb
+++ b/app/controllers/api/asciicasts_controller.rb
@@ -12,6 +12,8 @@ module Api
 
     rescue ActiveRecord::RecordInvalid => e
       render text: e.record.errors.messages, status: 422
+    rescue AsciicastParams::FormatError => e
+      render text: e.message, status: 400
     end
 
     def show
diff --git a/app/models/asciicast_params.rb b/app/models/asciicast_params.rb
index 7b6a5a1..8c10d1c 100644
--- a/app/models/asciicast_params.rb
+++ b/app/models/asciicast_params.rb
@@ -1,4 +1,5 @@
 class AsciicastParams
+  FormatError = Class.new(StandardError)
 
   def self.build(asciicast_params, username, token, user_agent)
     if asciicast_params.try(:respond_to?, :read)
@@ -39,11 +40,16 @@ class AsciicastParams
   end
 
   def self.from_format_1_request(asciicast_file, username, token, user_agent)
-    asciicast = Oj.sc_parse(AsciicastHandler.new, asciicast_file)
+    begin
+      asciicast = Oj.sc_parse(AsciicastHandler.new, asciicast_file)
+    rescue Oj::ParseError
+      raise FormatError, "This doesn't look like a valid asciicast file"
+    end
+
     version = asciicast['version']
 
     if version != 1
-      raise "unsupported asciicast format version: #{version}"
+      raise FormatError, "Unsupported asciicast format version: #{version}"
     end
 
     env = asciicast['env']
diff --git a/spec/api/asciicast_create_spec.rb b/spec/api/asciicast_create_spec.rb
index d35248e..204a702 100644
--- a/spec/api/asciicast_create_spec.rb
+++ b/spec/api/asciicast_create_spec.rb
@@ -307,9 +307,9 @@ describe "Asciicast creation" do
   context 'format 1' do
     subject { make_request }
 
-    def make_request
+    def make_request(asciicast_path = '1/asciicast.json')
       post '/api/asciicasts',
-        { asciicast: fixture_file('1/asciicast.json', 'application/json') },
+        { asciicast: fixture_file(asciicast_path, 'application/json') },
         headers('kill', 'f33e6188-f53c-11e2-abf4-84a6c827e88b', 'asciinema/1.0.0 gc/go1.3 jola-amd64')
     end
 
@@ -388,6 +388,23 @@ describe "Asciicast creation" do
     it 'returns the URL to the uploaded asciicast' do
       expect(response.body).to eq(asciicast_url(created_asciicast))
     end
+
+    context 'when json is missing data / has invalid data' do
+      subject { make_request('1/invalid.json') }
+
+      it 'returns 422 status' do
+        expect(response.status).to eq(422)
+      end
+    end
+
+    context 'when non-json file given' do
+      subject { make_request('stdout.decompressed') }
+
+      it 'returns 400 status' do
+        expect(response.status).to eq(400)
+      end
+    end
+
   end
 
 end
diff --git a/spec/fixtures/1/invalid.json b/spec/fixtures/1/invalid.json
new file mode 100644
index 0000000..ccfed30
--- /dev/null
+++ b/spec/fixtures/1/invalid.json
@@ -0,0 +1,16 @@
+{
+  "version": 1,
+  "height": 26,
+  "duration": 11.146430015564,
+  "command": "/bin/bash",
+  "title": "bashing :)",
+  "env": {
+    "TERM": "screen-256color",
+    "SHELL": "/bin/zsh"
+  },
+  "stdout": [
+    [1.234567, "foo bar"],
+    [5.678987, "baz qux"],
+    [3.456789, "żółć jaźń"]
+  ]
+}