Second Round CURL Wrapper Review

jdrewsen jdrewsen at
Fri Dec 30 14:03:38 PST 2011

On Saturday, 17 December 2011 at 22:25:07 UTC, Andrei 
Alexandrescu wrote:
> On 12/2/11 10:26 PM, dsimcha wrote:
>> I volunteered ages ago to manage the review for the second 
>> round of
>> Jonas Drewsen's CURL wrapper. After the first round it was 
>> decided that,
>> after a large number of minor issues were fixed, a second 
>> round would be
>> necessary.
>> Significant open issues:
>> 1. Should libcurl be bundled with DMD on Windows?
> Yes.
>> 2. etc.curl, std.curl, or (We had a vote a while 
>> back but
>> it was buried deep in a thread and a lot of people may have 
>> missed it:
>> )


>> Code: 
>> Docs:
> I have a few comments. They are not critical for library's 
> admission (apologies I didn't make the deadline) and could be 
> integrated before the commit.
> 1. Tables must be centered (this is a note to self)
> 2. Replace in examples with


> 3. "Compared to using libcurl directly this module provides a 
> simpler API for performing common tasks." -> "Compared to using 
> libcurl directly this module allows simpler client code for 
> common uses, requires no unsafe operations, and integrates 
> better with the rest of the language."


> 4. From the high level ops it seems there's no way to issue a 
> PUT/POST and then direct the result to a file, or read the 
> result with a foreach.

Use byLine/Chunk with a connection initialized with post/put data:

auto http = HTTP();
http.postData = "hello world";
foreach (line; byLine("",, '\n', 

Same deal for directing to file using download()

> 5. Also there seems to be no way to issue a PUT/POST from a 
> range.

Correct. It will be a later extension.

> 6. s/Examples:/Example:/


> 7. s/HTTP http = HTTP("");/auto http = 
> HTTP("");/


> 8. "receives a header or a data buffer" -> "receives a header 
> and a data buffer, respectively"


> 9. "In this simple example, the headers are writting to stdout 
> and the data is ignored." How can we make it to stop the 
> request? A sentence about that would be great.


> 10. "Finally the HTTP request is started by calling perform()." 
> -> "Finally the HTTP request is effected by calling perform(), 
> which is synchronous."


> 11. Use InferredProtocol instead of AutoConnect(ion). The type 
> is exactly what it claims. If "inferred" is too long, 
> "AutoProtocol" would be fine too. But it's not the connection 
> that is automated. The word must not be a verb, i.e. 
> InferProtocol/DeduceProtocol would not be good.


> 12. The example for InferredProtocol nee AutoConnection does 
> not show any interesting example, e.g. ftp.


> 13. I think download() and upload() are good convenience 
> functions but are very limited. Such operations can last 
> arbitrarily long and it's impossible to stop them. Therefore, 
> they will foster poor program design (e.g. program hangs until 
> you kill it etc). We should keep these for convenience in 
> scripts, and we already have byXxx() for downloading, but we 
> don't have anything for uploading. We should offer output 
> ranges for uploading sync/async. Maybe in the next minor 
> release.

Using the same method as for post it is possible to set timeouts.

auto http = HTTP();
http.dataTimeout = dur!"minutes"(2);
foreach (line; byLine("",, '\n', 

Regarding uploading ranges this will be handled by the same 
extension as
for POST/PUT output ranges.

> 14. Parameter doc for put(), post(), del() etc. is messed up.


> 15. I don't think "connection" is the right term. For example 
> we have
> T[] get(Conn = AutoConnection, T = char)(const(char)[] url, 
> Conn conn = Conn());
> The connection is established only during the call to get(), so 
> conn is not really a connection - more like a sort of a bundle 
> of connection parameters. But then it does clean up the 
> connection once established, so I'm unsure...

But you can provide an existing connection to use has that is 
already connected to the server. Hence the name connection.

> 16. For someone who'd not versed in HTTP options, the example
> string content = 
> options("", 
> "something");
> isn't terribly informative.

ok - improved

> 17. In byLine: "A range of lines is returned when the request 
> is complete." This suggests the last byte has been read as the 
> client gets to the first, which is not the case. "Data will be 
> read on demand as the foreach loop makes progress." Similar for 
> byChunk.

This will change when multi API is supported as well.

> 18. In byXxxAsync wait(Duration) should be cross-referenced to 
> its definition.


> =========================
> Overall: I think this is a valuable addition to Phobos, but I 
> have the feeling we don't have a good scenario for interrupting 
> connections. For example, if someone wants to offer a "Cancel" 
> button, the current API does not give them a robust option to 
> do so: all functions that transfer data are potentially 
> blocking, and there's no thread-shared way to interrupt a 
> connection in a thread from another thread.
> Such functionality may be a pure addition later, so I think we 
> can commit this as is. Please use

They can abort from the callback handlers - but otherwise it is a 
limitation of the libcurl easy API.
To actually do this the multi extension would have to be added.

Thank you for the comments and sorry for the late reply.


More information about the Digitalmars-d mailing list