Don't crash on invalid inline attachments

[eiri]

Overview

We trust inline attachment to be base64 encoded, so in case it's not provided or invalidly encoded the upload request crashes with function_clause.

The change catches decode attempt and re-throws it as 400 error.

Testing recommendations

Standard eunit and javascript tests should pass.

GitHub issue number

Fixes #784

Checklist

• Code is written and works correctly;
• Changes are covered by tests;
• Documentation reflects the changes;

[nickva]

In this case the data would be undefined so the case when data is not available at all in the request. Maybe have another clause where data is present but is invalid?

[eiri]

This covers the original's issue exception, but sure, I'll add another test.

[nickva]

Wonder if we should be more conservative and explicitly match on the knows exception tags and error message

error:function_clause
 
error:{badmatch, _}

the function_clause happens when is an invalid input data type such as undefined or list.

badmatch happens if type is correct but data is not a valid B64 encoding.

But I guess it's not clear that there couldn't be other errors thrown so maybe catching all is ok there. Docs don't say what exactly should be thrown.

So up to you here. I am 50/50 on this one.

[eiri]

I thought about it, but then decided to opt out. We don't particularly interested in how base64 can't decode our data, only in fact that it can't be decoded, so there are no merit to try and match all possible ways base64 can break, if at the end we are raising the same invalid request exception.

[nickva]

Yap got it, that makes sense

[nickva]

JS

make javascript suites=attachments.js
test/javascript/tests/attachments.js   pass
=======================================================
JavaScript tests complete.

EUnit

Compiled test/couch_changes_tests.erl
Compiled test/chttpd_endpoints_tests.erl
======================== EUnit ========================
module 'couch_att'
  Lazy record upgrade tests
    couch_att: attachment_upgrade_test_ (Existing record fields don't upgrade)...ok
    couch_att: attachment_upgrade_test_ (New fields upgrade)...ok
    [done in 0.006 s]
  Attachment defaults tests
    couch_att: attachment_defaults_test_ (Records retain old default values)...ok
    couch_att: attachment_defaults_test_ (Upgraded records inherit defaults)...ok
    couch_att: attachment_defaults_test_ (Undefined entries are elided on upgrade)...ok
    [done in 0.009 s]
  Basic attachment field api
    couch_att: attachment_field_api_test_...ok
    couch_att: attachment_field_api_test_...ok
    couch_att: attachment_field_api_test_...ok
    [done in 0.009 s]
  Disk term tests
    couch_att:789: attachment_disk_term_test_...ok
    couch_att:790: attachment_disk_term_test_...ok
    couch_att:791: attachment_disk_term_test_...ok
    couch_att:792: attachment_disk_term_test_...ok
    [done in 0.012 s]
  JSON term tests
    couch_att:822: attachment_json_term_test_...[0.009 s] ok
    couch_att:823: attachment_json_term_test_...ok
    couch_att:824: attachment_json_term_test_...ok
    couch_att:825: attachment_json_term_test_...ok
    couch_att:826: attachment_json_term_test_...ok
    [done in 0.024 s]
  Attachment stub merging tests
  [done in 0.060 s]
=======================================================
  All 17 tests passed.
==> rel (eunit)
==> asf4 (eunit)

[eiri]

@nickva Had to revert spec restoration commit, apparently R16 still doesn't know about dict() type

[nickva]

Ah ok, cool. Still approved, merge it when ready.