Support file attachments on HTTPSFuture
ClosedPublic
Actions

Authored by epriestley on Apr 21 2014, 6:40 PM.

Details

Reviewers

Commits

rPHU159b5065d9d4: Support file attachments on HTTPSFuture

Summary

@zeeg wants this for some build stuff and we'd like it in general to make Mailgun work better. It's a bit messy because of how cURL works. See pages of comments inline.

Test Plan

Added tests for the parser stuff.
Made various requests with and without file attachments and saw them encode data as expected.
Made some dangerous requests (with "@") and saw them fail.

Diff Detail

Repository

rPHU libphutil

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

epriestley updated this revision to Diff 20953.Apr 21 2014, 6:40 PM

epriestley retitled this revision from to Support file attachments on HTTPSFuture.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: btrahan.

epriestley added a subscriber: zeeg.

Herald added a subscriber: epriestley. · View Herald TranscriptApr 21 2014, 6:40 PM

Harbormaster completed remote builds in B32: Diff 20953.Apr 21 2014, 6:42 PM

Handle mime types and file names; use CURLFile if available.

Harbormaster completed remote builds in B33: Diff 20954.Apr 21 2014, 7:12 PM

Are we locked into supporting certain PHP versions? Diff had me wondering.

This revision is now accepted and ready to land.Apr 21 2014, 9:26 PM

We don't formally support PHP before 5.2.3, but we don't have a hard kill for earlier versions in libphutil (we do in Phabricator).

I think the major risk here is someone including the library, or copy/pasting the code, or using it as a reference to implement something similar. In most cases, if you do that and run on an old PHP you get a nonfunctional feature which is broken in an obvious way (e.g., missing function or something like that), but in this case you'd get a gaping security hole. The two lines of cruft feel kind-of-okayish to me given the extreme badness of not doing the check.