Journal Articles
| Browse in : |
All
> Journals
> CVu
> 304
(10)
All > Topics > Programming (877) All > Journal Columns > Code Critique (70) Any of these categories - All of these categories |
Note: when you create a new publication type, the articles module will automatically use the templates user-display-[publicationtype].xt and user-summary-[publicationtype].xt. If those templates do not exist when you try to preview or display a new article, you'll get this warning :-) Please place your own templates in themes/yourtheme/modules/articles . The templates will get the extension .xt there.
Title: Code Critique Competition
Author: Bob Schmidt
Date: 03 September 2018 18:22:33 +01:00 or Mon, 03 September 2018 18:22:33 +01:00
Summary: Set and collated by Silas Brown. A book prize is awarded for the best entry.
Body:
Please note that participation in this competition is open to all members, whether novice or expert. Readers are also encouraged to comment on published entries, and to supply their own possible code samples for the competition (in any common programming language) to scc@accu.org.
Note: if you would rather not have your critique visible online please inform me. (Email addresses are not publicly visible.)
Last issue’s code
Further to articles introducing D, I am attempting to use the event-driven Vibe.d server library, which I understand to be like a native version of Python’s Tornado and potentially a ‘killer app’ of D as I haven’t seen any other mature event-driven server library in a compiled language.
I would like to build a back-end server that performs some processing on the body of the HTTP request it receives. To begin with, I would like it to simply echo the request body back to the client.
My code works but there are three problems: (1) when I issue a POST request with lynx, 3 spaces are added to the start of the response text, (2) I cannot test with nc because it just says 404 and doesn’t log anything, and (3) I’m worried that reading and writing just one byte at a time is inefficient, but I can’t see how else to do it: I raised a ‘more documentation needed’ bug at https://github.com/vibe-d/vibe.d/issues/2139 but at the time of writing nobody has replied (should I have used a different forum?).
The code is in Listing 1.
import vibe.d;
void main() {
auto settings = new HTTPServerSettings;
settings.port = 8080;
listenHTTP(settings, (req, res) {
ubyte[1] s;
while(!req.bodyReader.empty()) {
req.bodyReader.read(s);
res.bodyWriter.write(s);
}
});
runApplication();
}
|
| Listing 1 |
Critiques
Russel Winder <russel@winder.org.uk>
The statement of the problem gives some D code using the framework Vibe.d (a single-threaded asynchronous fibres framework with support for HTTP and HTTPS processing), but doesn’t indicate how the code is to be compiled. One could, I suppose, use one of the three D compilers (dmd, ldc2, or gdc) directly, or use Meson or SCons, but you would have to have already sorted out the dependency on Vibe.d to do that. SCons has some vestigial support for handling dependencies, but it isn’t in production as yet. The standard way of handling dependencies and build in the development of D projects is to use Dub. So to start, let us actually build the presented code.
It is worth noting that the original problem code is an amendment of the first example of using Vibe.d at https://github.com/vibe-d/vibe.d. That ‘Hello Vibe.d’ server is not really an echo, though; it is just writing a message constructed of a literal and some data from the request data structure.
It is also worth noting, that the question raised at https://github.com/vibe-d/vibe.d/issues/2139 has been answered, albeit still open. It was certainly the right thing to raise this issue.
In working on this code, I worked with Debian Sid using Emacs (obviously, though there is the D plugin for IntelliJ IDEA, which perhaps I should have used) and D 2.081.1. I used both the dmd and ldc compilers: dmd 2.081.1 was installed using packages from the D-Apt repository. ldc 1.11.0 is not yet formally released yet, so I built ldc 1.11-beta from a clone of the Git repository – it’s really quite easy as long as you have a D compiler installed, Debian current has ldc 1.8.0 which is ancient, but it if fine for the job of bootstrapping the ldc build.
Dubbing it
There are two notations for creating Dub specification files: JSON and SDL. Personally I prefer SDL, even though a lot of people use JSON. See https://code.dlang.org/getting_started
There is a canonical project organisation for a D project: all the source is in the subdirectory source, and the entry point (the main function) of an application is either in the file source/main.d or source/app.d. Without a file with one of these names or explicit specification, the project is assumed to be a library. Using the canonical structure means an almost empty specification file when using Dub.
So with the code from original code author’s statement in the file source/main.d, I created a dub.sdl file:
name "server" description "A sample server using vibe.d for CVu Code Critique 112" dependency "vibe-d" version="*"
Note that I have used "*" as the version number for the vibe-d dependency here, which means use the latest version you can find in the Dub packages repository at http://code.dlang.org/. I could have written "0.8.4" to specify a specific version. The version descriptions possible are outlined at http://code.dlang.org/package-format?lang=sdl#version-specs
So with the Dub project specification file in place, all that is required to build the project is to run the command:
dub build
The result was for Dub to download the specified dependency and all transitive dependencies and compile them:
|> dub build --compiler=$HOME/BuildArea/LDC/bin/ ldc2 Fetching eventcore 0.8.35 (getting selected version)... Fetching libevent 2.0.2+2.0.16 (getting selected version)... Fetching diet-ng 1.5.0 (getting selected version)... Fetching taggedalgebraic 0.10.11 (getting selected version)... Fetching openssl 1.1.6+1.0.1g (getting selected version)... Fetching botan 1.12.10 (getting selected version)... Fetching stdx-allocator 2.77.2 (getting selected version)... Fetching vibe-d 0.8.4 (getting selected version)... Fetching mir-linux-kernel 1.0.0 (getting selected version)... Fetching memutils 0.4.11 (getting selected version)... Fetching vibe-core 1.4.1 (getting selected version)... Fetching libasync 0.8.3 (getting selected version)... Fetching botan-math 1.0.3 (getting selected version)... Performing "debug" build using /home/users/ russel/BuildArea/LDC/bin/ldc2 for x86_64. taggedalgebraic 0.10.11: building configuration "library"... eventcore 0.8.35: building configuration "epoll"... stdx-allocator 2.77.2: building configuration "library"... vibe-core 1.4.1: building configuration "epoll"... vibe-d:utils 0.8.4: building configuration "library"... vibe-d:data 0.8.4: building configuration "library"... mir-linux-kernel 1.0.0: building configuration "library"... vibe-d:crypto 0.8.4: building configuration "library"... diet-ng 1.5.0: building configuration "library"... vibe-d:stream 0.8.4: building configuration "library"... vibe-d:textfilter 0.8.4: building configuration "library"... vibe-d:inet 0.8.4: building configuration "library"... vibe-d:tls 0.8.4: building configuration "openssl-1.1"... vibe-d:http 0.8.4: building configuration "library"... vibe-d:mail 0.8.4: building configuration "library"... vibe-d:mongodb 0.8.4: building configuration "library"... ../../../../../.dub/packages/vibe-d-0.8.4/vibe-d/ mongodb/vibe/db/mongo/settings.d(16,8): Deprecation: alias `std.digest.digest.toHexString` is deprecated – import std.digest instead of std.digest.digest. std.digest.digest will be removed in 2.084 ../../../../../.dub/packages/vibe-d-0.8.4/vibe-d/ mongodb/vibe/db/mongo/settings.d(16,8): Deprecation: alias `std.digest.digest.toHexString` is deprecated – import std.digest instead of std.digest.digest. std.digest.digest will be removed in 2.084 vibe-d:redis 0.8.4: building configuration "library"... vibe-d:web 0.8.4: building configuration "library"... vibe-d 0.8.4: building configuration "vibe- core"... server ~master: building configuration "application"...
We can ignore the deprecation warning as it is nothing to with our code, and it is a warning. Let’s hope the thing gets fixed fairly soon, D 2.084.0 is not that far distant given we are on 2.081.1 now.
Note that the compiler has been explicitly specified. If --compiler=… is not specified then Dub looks for D compilers in the path using the order dmd, ldc2, gdc. I ran first with my build of ldc2 1.11-beta and show the output here. I then also ran without the --compile=… and the dmd 2.081.1 was package installed and used.
All the dependency compilation products are cached:
|> dub build --compiler=$HOME/BuildArea/LDC/bin/ ldc2 Performing "debug" build using /home/users/ russel/BuildArea/LDC/bin/ldc2 for x86_64. taggedalgebraic 0.10.11: target for configuration "library" is up to date. eventcore 0.8.35: target for configuration "epoll" is up to date. stdx-allocator 2.77.2: target for configuration "library" is up to date. vibe-core 1.4.1: target for configuration "epoll" is up to date. vibe-d:utils 0.8.4: target for configuration "library" is up to date. vibe-d:data 0.8.4: target for configuration "library" is up to date. mir-linux-kernel 1.0.0: target for configuration "library" is up to date. vibe-d:crypto 0.8.4: target for configuration "library" is up to date. diet-ng 1.5.0: target for configuration "library" is up to date. vibe-d:stream 0.8.4: target for configuration "library" is up to date. vibe-d:textfilter 0.8.4: target for configuration "library" is up to date. vibe-d:inet 0.8.4: target for configuration "library" is up to date. vibe-d:tls 0.8.4: target for configuration "openssl-1.1" is up to date. vibe-d:http 0.8.4: target for configuration "library" is up to date. vibe-d:mail 0.8.4: target for configuration "library" is up to date. vibe-d:mongodb 0.8.4: target for configuration "library" is up to date. vibe-d:redis 0.8.4: target for configuration "library" is up to date. vibe-d:web 0.8.4: target for configuration "library" is up to date. vibe-d 0.8.4: target for configuration "vibe- core" is up to date. server ~master: target for configuration "application" is up to date. To force a rebuild of up-to-date targets, run again with --force.
The version numbers of the dependencies currently compiled are stored in the file dub.selections.json. If you have not specified exact versions of the dependencies and you want to check for updates, you run dub upgrade and, if there are new versions, the dependency versions will be updated, the new versions downloaded, and then compiled.
Running it
The upshot of all the previous section is that we have a compiled program, and so we can try it out:
|> ./server [main(----) INF] Listening for requests on http://[::]:8080/ Failed to listen on 0.0.0.0:8080
If it isn’t listening in 0.0.0.0:8080, is it listening on anything? Well the way forward is to try and see. When I navigated to http://localhost:8080 using a browser, in this case Firefox, it reports:
404 – Not Found Not Found Internal error information: No routes match path '/'
which seems to indicate something is listening on 127.0.0.1:8080, but something that can’t do an HTTP GET of /. The original code author did say though that they used lynx to issue a POST, but doesn’t say what command line they used. I don’t have lynx, but I do have curl on my computers, so I shall s/lynx/curl/ and try an experiment.
|> curl --data-binary "Hello World" http://localhost:8080 Hello World
OK, so the original code author’s claim is confirmed, it is echoing HTTP POST data. Phew. <Bullet>
?</Bullet>
Except, I am not seeing three leading spaces added to the return. Should I smell a rat or is this perhaps a ‘Lynx Effect’?
As for reading and writing a single byte at a time, the person answering the original code author’s issue at https://github.com/vibe-d/vibe.d/issues/2139 states that a canonical loop should look like:
while (!conn.empty) {
ubyte[256] buf = void;
auto n = conn.read(buf[], IOMode.once);
dst.write(buf[0 .. n]);
}
so I thought I should try that.
Updated version
Well, except, I didn’t. The conn and dst in the code fragment raised a ‘flag’. The delegate (aka lambda function) that is the function passed into the listenHTTP function in the original code is written to take parameters req and res of inferred type. This seems like HTTP request data and HTTP result data, which doesn’t seem like a TCP connection type thing as inferred from the name conn.
Delving into the examples from the Vibe.d GitHub repository, we find at https://github.com/vibe-d/vibe.d/blob/master/examples/echoserver/source/app.d:
import vibe.appmain;
import vibe.core.net;
import vibe.core.stream;
shared static this()
{
listenTCP(2000, (conn) {
try conn.pipe(conn);
catch (Exception e) conn.close();
});
}
which I think we can assume is the canonical echo server written in D using Vibe.d. Notice it uses pipe, a function that is mentioned in the reply at https://github.com/vibe-d/vibe.d/issues/2139.
Interesting code. The shared static this() is the module initialisation block, executed when the executable starts, before main is executed. Why no main function in the source? Because it is boilerplate. import vibe.appmain imports the needed main.
NB For C and C++ folk: there are no #include statements because D is a module-based language. No textual code inclusion, just access to other modules via import statements. Most modern. <Bullet>?</Bullet>
Obviously, though, this is a TCP server, not an HTTP server. If the goal was to have a packet echo server then ‘Job Done’. However, the original code author emphasised that HTTP POSTs were expected.
POSTing version
Let us look again at the examples from the Vibe.d GitHub repository. We immediately find an HTTP server (https://github.com/vibe-d/vibe.d/blob/master/examples/http_server/source/app.d) and an HTTPS server (https://github.com/vibe-d/vibe.d/blob/master/examples/https_server/source/app.d). Clearly, we should only be considering HTTPS for production code. However, as can be seen from the two example listings, an HTTPS server is an HTTP server with added settings. So, for experimentation we can simplify by working with HTTP, but then ensure we add the extra settings and keys to use HTTPS before going into production.
So, based on the HTTP server example (so it can be tried without creating keys etc.) here is a bit of code that responds to GETs with a message so as to be certain the right server has been reached and that echoes POST message bodies.
import vibe.appmain;
import vibe.http.server;
void handleRequest(scope HTTPServerRequest req,
scope HTTPServerResponse res) {
if (req.method == HTTPMethod.POST) {
res.writeBody(req.bodyReader.peek(),
"text/plain");
} else {
res.writeBody("I got a GET.\n",
"text/plain");
}
}
shared static this() {
auto settings = new HTTPServerSettings;
settings.port = 8080;
settings.bindAddresses = ["::1",
"127.0.0.1"];
listenHTTP(settings, &handleRequest);
}
This I would claim is fairly canonical Vibe.d use. Use the default Vibe.d main, set up all the server setting in the module initialisation, with a top level function that is the callback for all server requests.
In the handler, control is split for POST and GET requests. In the GET request branch we just write a valid response using methods provided by the HTTPServerResponse instance res. In the POST request branch, we get a reference to the buffer in the HTTPServerRequest instance req that holds the POSTed data and construct a response using that. Seems like the very essence of echo.
Comparing this to the original code, and perhaps explaining why it was wrong, and this version at least better: the crux of this version is using:
HTTPServerResponse.writeBody
instead of:
HTTPServerResponse.bodyWriter.write
and:
HTTPServerRequest.bodyReader.peek
to access the request body rather than using
HTTPServerRequest.bodyReader.read
to read it byte at a time. By using the higher level functions, any looping is implicit, so no explicit while statement – more declarative.
To compile this version and show it meets the requirements, we must add a line to the dub.sdl file so that it reads:
name "server" description "A sample server using vibe.d for CVu Code Critique 112" dependency "vibe-d" version="*" versions "VibeDefaultMain"
The default main system requires the use of conditional compilation D style. So we set the compilation version to VibeDefaultMain and now everything compiles and indeed runs:
|> dub run --compiler=$HOME/BuildArea/LDC/bin/ ldc2 Performing "debug" build using /home/users/ russel/BuildArea/LDC/bin/ldc2 for x86_64. taggedalgebraic 0.10.11: building configuration "library"... eventcore 0.8.35: building configuration "epoll"... stdx-allocator 2.77.2: building configuration "library"... vibe-core 1.4.1: building configuration "epoll"... vibe-d:utils 0.8.4: building configuration "library"... vibe-d:data 0.8.4: building configuration "library"... mir-linux-kernel 1.0.0: building configuration "library"... vibe-d:crypto 0.8.4: building configuration "library"... diet-ng 1.5.0: building configuration "library"... vibe-d:stream 0.8.4: building configuration "library"... vibe-d:textfilter 0.8.4: building configuration "library"... vibe-d:inet 0.8.4: building configuration "library"... vibe-d:tls 0.8.4: building configuration "openssl-1.1"... vibe-d:http 0.8.4: building configuration "library"... vibe-d:mail 0.8.4: building configuration "library"... vibe-d:mongodb 0.8.4: building configuration "library"... ../../../../../.dub/packages/vibe-d-0.8.4/vibe-d/ mongodb/vibe/db/mongo/settings.d(16,8): Deprecation: alias `std.digest.digest.toHexString` is deprecated – import std.digest instead of std.digest.digest. std.digest.digest will be removed in 2.084 ../../../../../.dub/packages/vibe-d-0.8.4/vibe-d/ mongodb/vibe/db/mongo/settings.d(16,8): Deprecation: alias `std.digest.digest.toHexString` is deprecated – import std.digest instead of std.digest.digest. std.digest.digest will be removed in 2.084 vibe-d:redis 0.8.4: building configuration "library"... vibe-d:web 0.8.4: building configuration "library"... vibe-d 0.8.4: building configuration "vibe- core"... server ~master: building configuration "application"... Running ./server [main(----) INF] Listening for requests on http://[::1]:8080/ [main(----) INF] Listening for requests on http://127.0.0.1:8080/
and now if we curl things in:
|> curl http://localhost:8080
I got a GET.
|> curl --data-binary "Hello World"
http://localhost:8080
Hello World
Is it now time to say ‘Job Done’? Probably.
Addendum
Having submitted my response to Code Critique 112 to the appropriate authorities, it seems the original problem setter reviewed the piece and came up with a problem. From the email:
Unfortunately, though, his revised POST-handling code works with small requests but not with large ones. On my system:
curl --data-binary $(yes | head -300)
http://localhost:8080 | wc -l
299
(this is OK: the 300th line has no \n terminator as per HTTP POST semantics), but
curl --data-binary "$(yes|head -3000)"
http://localhost:8080 | wc -l
0
(this is not OK.) I think the reason for this is the reliance on peek(), the documentation of which says “Returns a temporary reference to the data that is currently bufferedâ€. There is no guarantee that ALL the data will currently be buffered, especially if there’s a lot of it.
This is not only entirely correct, it highlights that I failed to do proper system testing. I shall, of course, take myself immediately off into a corner and tear strips off myself until I am appropriately contrite.
Some minutes later:
So, first I must reproduce the error (see Figure 1).
|> curl --data-binary "$(yes | head -300)" http://localhost:8080 | wc -l
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 1198 100 599 100 599 42785 42785 --:--:-- --:--:-- --:--:-- 85571
299
|> curl --data-binary "$(yes | head -3000)" http://localhost:8080 | wc -l
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 5999 0 0 100 5999 0 976k --:--:-- --:--:-- --:--:-- 976k
0
|
| Figure 1 |
So now to deal with the problem of peek. That algorithm made the, clearly unreasonable, assumption that data would always come in a single buffer; without delving into the implementation, it is not clear what the exact problem is, but we know using peek doesn’t work as tried in the submitted code.
Original problem setter goes on to say:
The answer on Vibe.d issue 2139 said that a canonical read loop would be:
while (!conn.empty) {
ubyte[256] buf = void;
auto n = conn.read(buf[], IOMode.once);
dst.write(buf[0 .. n]);
}
so I tried changing the HTTPMethod.POST branch to:
ubyte[] rBody;
while (!req.bodyReader.empty) {
ubyte[256] buf = void;
auto n = req.bodyReader.read(buf[],
IOMode.once);
rBody ~= buf[0 .. n];
}
// (optionally do processing on rBody here)
res.writeBody(rBody, "text/plain");
and adding import vibe.d; at the start, but the result was the message “413 – Request Entity Too Large†even with just a single-byte argument in curl’s --data-binary option. This doesn’t seem to make much sense: why would it let you see (small) requests when you peek(), but send the client an error when you read()? I suppose we will have to wait for Vibe.d to add more beginner-friendly documentation.
So, I should test this updated code (see Figure 2 for the results):
import vibe.appmain;
import vibe.http.server;
import eventcore.driver: IOMode;
void handleRequest(scope HTTPServerRequest req,
scope HTTPServerResponse res) {
if (req.method == HTTPMethod.POST) {
ubyte[] rBody;
while (!req.bodyReader.empty) {
ubyte[256] buf = void;
auto n = req.bodyReader.read(
buf, IOMode.once);
rBody ~= buf[0 .. n];
}
res.writeBody(rBody, "text/plain");
} else {
res.writeBody("I got a GET.\n",
"text/plain");
}
}
shared static this() {
auto settings = new HTTPServerSettings;
settings.port = 8080;
settings.bindAddresses = ["::1",
"127.0.0.1"];
listenHTTP(settings, &handleRequest);
}
|> curl --data-binary "$(yes | head -300)" http://localhost:8080 | wc -l
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 655 100 56 100 599 9333 99833 --:--:-- --:--:-- --:--:-- 106k
2
|> curl --data-binary "$(yes | head -3000)" http://localhost:8080 | wc -l
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 6055 100 56 100 5999 11200 1171k --:--:-- --:--:-- --:--:-- 1182k
2
|> ll
|
| Figure 2 |
Not the result original problem setter got, but still wrong. In fact, the original problem setter’s result is obtained if the | wc -l is removed. Obvious really, but it took me a moment:
|> curl --data-binary "$(yes | head -300)" http://localhost:8080 413 - Request Entity Too Large Request Entity Too Large |> curl --data-binary "$(yes | head -3000)" http://localhost:8080 413 - Request Entity Too Large Request Entity Too Large
It seemed likely that this is the result of an exception, probably in the req.bodyReader.read. Experimenting with a try/catch block indicated this was indeed the case. So it seems there is some tension here between req.bodyReader.empty being false and yet req.bodyReader.read having a problem.
Despite the manual entry https://vibed.org/api/vibe.core.stream/InputStream.read, it would seem that this function always tries to read to fill all of the buffer and if the stream ends before the read is complete an exception is thrown. One could argue, given the signature of the function, that this behaviour is wrong, and indeed broken: the read should happen until the end of stream and the number of items read returned.
Anger.
I posted a message to the D Learn email list and put a post on the vibe.d forum, even though I hate forums. A most splendid person on the D Learn email list took up my case and checked the source code. It seems that the code does not implement the documentation; the behaviour I saw is the expected behaviour given the source code. However, this person also said that no active vibe.d user would be using these methods so, in a sense, the bug isn’t all that surprising because no-one is exercising them. It does, though, imply the vibe.d tests are a bit short of coverage of public API.
The person did also assure me that there are higher level ways of working and that is what all vibe.d users would actually be doing, that these all work exactly as documented and, indeed, as desired. I shall take it upon myself to try and get that person to write a follow up article to this Code Critique to be published in CVu.
A bit more anger.
Some hours later:
So clearly I have to take vibe.d as it is, not as it is documented. In this spirit, I have to:
- catch the exception indicating we have got to the end of the stream representing the request body,
- find out how many
ubytes are left to read, - create an appropriate size buffer,
- read the remaining
ubytes, - add these to the response body.
So now I have:
import vibe.appmain;
import vibe.http.server;
import eventcore.driver: IOMode;
void handleRequest(scope HTTPServerRequest req,
scope HTTPServerResponse res) {
if (req.method == HTTPMethod.POST) {
ubyte[] resBody;
enum bufferSize = 256;
while (!req.bodyReader.empty) {
ubyte[bufferSize] buffer;
// req.bodyReader.read does not, as
// of 0.8.4, behave as documented.
// The function either completely
// fills the buffer or throws an
// exception.
try {
auto n = req.bodyReader.read(
buffer, IOMode.once);
assert(n == bufferSize);
resBody ~= buffer[0 .. $];
} catch (Throwable t) {
// There were not 256 items to
// read, so lets use a deprecated
// method to get what we need. No
// idea what we'll be able to do
// after this property gets
// removed.
auto count =
req.bodyReader.leastSize;
assert(count < bufferSize);
ubyte[] bufferSlice =
buffer[0 .. count];
auto n = req.bodyReader.read(
bufferSlice, IOMode.once);
assert(n == count);
resBody ~= bufferSlice[0 .. $];
break;
}
}
res.writeBody(resBody, "text/plain");
} else {
res.writeBody("I got a GET.\n",
"text/plain");
}
}
shared static this() {
auto settings = new HTTPServerSettings;
settings.port = 8080;
settings.bindAddresses = ["::1",
"127.0.0.1"];
listenHTTP(settings, &handleRequest);
}
Of course, the first thing has to be to test this with original problem setters test (see Figure 3).
|> curl --data-binary "$(yes | head -300)" http://localhost:8080 | wc -l
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 1198 100 599 100 599 292k 292k --:--:-- --:--:-- --:--:-- 584k
299
|> curl --data-binary "$(yes | head -3000)" http://localhost:8080 | wc -l
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 11998 100 5999 100 5999 2929k 2929k --:--:-- --:--:-- --:--:-- 5858k
2999
|
| Figure 3 |
Exciting, this seems to be the right result. Let me test the earlier test to make sure that hasn’t broken:
|> curl http://localhost:8080 I got a GET. |> curl --data-binary "Hello World" http:// localhost:8080 Hello World
Am I done? Well in one sense yes, I dealt with original problem setters problem with the previously submitted code, in the process uncovering something not entirely good with a bit of the vibe.d API. However, the code is a hack, it shouldn’t have to be like this.
If I have to use deprecated features anyway to solve the problem, why not go for it and put the deprecated feature at the heart of the code now so as to make the code as uncomplicated as possible. Certainly this leaves the problem of dealing with the lack of feature when it is removed, but when that happens maybe the API will be different in the light of bugs and lobbying?
So the plan is:
- remove the fixed size buffer, use dynamically allocated buffers always,
- arrange that the call to req.bodyReader.read so it always reads as much as it can and no more, no less,
- remove the try/catch since there should never be a read shorter than the buffer.
This leads to the code:
import vibe.appmain;
import vibe.http.server;
import eventcore.driver: IOMode;
void handleRequest(scope HTTPServerRequest req,
scope HTTPServerResponse res) {
if (req.method == HTTPMethod.POST) {
ubyte[] resBody;
while (!req.bodyReader.empty) {
//NB The leastSize property is deprecated.:-(
auto buffer = new
ubyte[req.bodyReader.leastSize];
auto n = req.bodyReader.read(
buffer, IOMode.once);
resBody ~= buffer[0..$];
}
res.writeBody(resBody, "text/plain");
} else {
res.writeBody("I got a GET.\n",
"text/plain");
}
}
shared static this() {
auto settings = new HTTPServerSettings;
settings.port = 8080;
settings.bindAddresses = ["::1",
"127.0.0.1"];
listenHTTP(settings, &handleRequest);
}
Does it pass the tests I have? (See Figure 4 to find out.)
|> curl http://localhost:8080 I got a GET. |> curl --data-binary "Hello World" http://localhost:8080 Hello World
|> curl --data-binary "$(yes | head -300)" http://localhost:8080 | wc -l
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 1198 100 599 100 599 116k 116k --:--:-- --:--:-- --:--:-- 233k
299
|> curl --data-binary "$(yes | head -3000)" http://localhost:8080 | wc -l
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 13901 100 7902 100 5999 1543k 1171k --:--:-- --:--:-- --:--:-- 2715k
2999
|
| Figure 4 |
Seems like a yes. The code seems very much nicer expressed like this. It’s just that it relies on deprecated features. I think this presages a lobbying campaign to find out why the feature is deprecated and to reverse that decision before the feature actually gets removed.
Obviously, it may be that a vibe.d expert could tell us why this solution is bad and what the right solution should be.
[Editor’s note: Russel goes on to say that Sönke (the leader of the vibe.d project) has written on the forum that the behaviour of read(buffer, IOMode) we have seen does indeed seem like a bug. It is going to be investigated. However, it turns out not to be a trivial thing to fix because of some things. Russel has posted an issue on all this on GitHub:
https://github.com/vibe-d/vibe.d/issues/2194 ]
Jason Spencer <contact@jasonspencer.org>
This is my first foray into D, but I’ll give it a go…
Firstly, I absolutely agree with ‘killer app’ – because the different compiler versions, package versions, too-clever-for-their-own-good build systems may well end up killing somebody. They tried with me.
Addressing your problems in order:
Post with Lynx
The reason for the three spaces is that Lynx is an interactive browser and attempts to format the response in a more readable way. To disable this and just see the unformatted response, use the -source switch (see Figure 5).
$ echo -n "WIBBLE" | lynx http://127.0.0.1:8080 --post_data|hexdump -C 00000000 20 20 20 57 49 42 42 4c 45 0a 0a | WIBBLE..|0000000b $ echo -n "WIBBLE" | lynx http://127.0.0.1:8080 --post_data -source|hexdump -C 00000000 57 49 42 42 4c 45 |WIBBLE|00000006 |
| Figure 5 |
Alternatively use wget [1] or curl [2] for command line HTTP clients.
404 with nc
The thing to note about nc is that it’s not an HTTP client but a command line tool for creating raw TCP or UDP sockets (or listening sockets). So we have to send the correct HTTP request with all the headers and the correct delimitation. Without seeing the original nc command, it’s hard to know why the response was 404.
Using Hobbit’s original netcat [3] something like this works:
$ echo -en "POST / HTTP/1.1\r\nUser-Agent: CVu FTW\r\nAccept: \*/\*\r\nAccept-Encoding: identity\r\nHost: 127.0.0.1:8080\r\nContent- Length:6\r\n\r\nWIBBLE" | nc -i 3 -v 127.0.0.1 8080 Connection to 127.0.0.1 8080 port [tcp/http-alt] succeeded! HTTP/1.1 200 OK Server: vibe.d/1.4.1 Date: Tue, 31 Jul 2018 21:00:07 GMT Keep-Alive: timeout=10 Transfer-Encoding: chunked 6 WIBBLE 0
Things to note here are that we’re using a POST command. Also, we’re using CR-LF (/r/n) line endings. The strange 6 and 0 in the response are because by default vibe.d sends chunked responses (as per the Transfer-Encoding header), and for good reason. The 6 and the 0 are both the number of bytes about to be sent. The 0 indicates the end of output. Chunking is often used when it’s not known how much data is to be sent – and since vibe.d doesn’t buffer the entire response before sending it, at least when you’re using streams, it doesn’t know the size of the whole response. To write the response in one go, try:
HTTPServerResponse.writeBody.
Echoing efficiency
It is hard to understand what the program is supposed to do exactly – is it to reply to GET requests? In which case, why read the body? There is no body in a GET request. If it is the student’s intention to echo the headers, req.bodyReader will only access the contents of the body, and not the headers. The headers can be read through the req.headers dictionary.
There are at least two ways to make this more efficient, though. Firstly, the array can be made bigger, say 65536 bytes, and a call to InputStream.read ( scope ubyte[] dst, IOMode mode ) will return the number of bytes actually read into dst (there could be less than 65536 bytes available). You can then write this many to the response output stream. You can manipulate the body data before sending it, but beware of encoding types and that the data will be presented in fixed blocks, and not line delimited, for example.
Alternatively, if all we want to do is echo back the POSTed body, we can link the request body input stream with the response body output stream with a pipe:
import std.stdio;
import vibe.d;
shared static this()
{
auto settings = new HTTPServerSettings;
settings.port = 8080;
listenHTTP(settings, (req, res) {
stdout.writef( "Got %d request type from %s\n",
req.method, req.clientAddress.toString() );
auto N = pipe(req.bodyReader,res.bodyWriter);
stdout.writef( "piped %d bytes\n", N);
});
}
The echoing of the body only really makes sense when the request type is a POST, but for debug purposes I’m printing the type to the console. This value can be checked against the vibe.http.common.HTTPMethod Enum to see which type it is.
Also, consider including a Content-Type header to match that of the request – if vibe.d assumed the type, and the request is a different type, then the client making the request will just get confused. The header can be accessed through HTTPServerRequest.contentType.
Note that I’ve switched from using main() to this() as newer vibe.d versions provide a main by default and we just need to state what we want the application to do.
I think that’s all, without a better idea of what the code was supposed to do. The problems were mostly actually in understanding HTTP and the various tools. [4] From what I’ve seen, Vibe.d works ok, but still has a few rough edges, and the documentation, as mentioned, is not great.
In regard to the student’s comment about a mature event-driven server library in a compiled language, perhaps it’s not been around as long as vibe.d but there is Boost Beast [5] for a C++ equivalent.
References
[1] https://www.gnu.org/software/wget/
[3] http://nc110.sourceforge.net/
[4] https://tools.ietf.org/html/rfc2616
[5] http://www.boost.org/libs/beast
Commentary
Jason was able to address the Lynx issue, but he didn’t quite understand the original problem. That’s partly my fault for not having stated it more clearly. The key sentence was “I would like to write a back-end server that performs some processing on the body of the HTTP request it receives.†The phrase “body of the HTTP request†means it must be a POST request, and the “echo the request body back to the client†goal was just a ‘get it up and running’ prelude to adding the extra processing. Perhaps I should have given an example: “I want to write a back-end server that takes a POST request of some English text and returns a translation into French, but, before I start on that massively clever machine-translation function, I’d like to return the English as-is, just to get the basic server infrastructure working.â€
From this it can be inferred that pipe() is not really an adequate solution because, although it technically does what was asked for, it doesn’t have an obvious hook where I can add my "text = my_clever_translation_function(text)" later. I do, however, appreciate Jason’s suggestion of looking at Vinnie Falco’s Boost.Beast C++ library – Boost.Beast does seem to require a LOT more user code than Vibe.d, but at least Boost.Beast currently seems to be better documented with good examples to get you going.
Russel was able to get his submission in early and look into fixing the problem that came up with the use of peek(). (No need to tear strips off himself though: we all make mistakes!) His use of the D Learn email list and Vibe.d forum is a particularly good example for a beginner because it shows where more help can be found. It’s a pity that Vibe.d does not work as documented and he ended up having to use deprecated features, but it’s probably the best thing that could be done within the time constraints, and it resulted in issues being opened and looked at, which has got to be a positive outcome.
Meanwhile I decided not to rewrite my Python Tornadoweb-based server into Vibe.d (unless a really good forthcoming article changes my mind); instead I improved the existing server’s responsiveness by using Python’s ctypes library to access a string-processing function I had written in C. Before using ctypes, the best I could do was shell out to the command line to pipe text through my C program on a per-request basis, which meant there was a process-launch delay for every request. But when I used ctypes, that function became part of a shared library that was preloaded into the Python process, eliminating the per-request load delay.
The Winner of CC 112
It has to be Russel, for his engagement with (and prompting improvement in) the D community, and for coming up with code that can clearly be used as a starting point for arbitrary processing of the submitted text. Jason’s effort deserves an honourable mention, though.
Code Critique 113
(Submissions to scc@accu.org by Oct 1st)
This is a very short code sample but nonetheless an interesting problem.
The writer is using a variadic template to pass multiple arguments to the least function, which uses std::sort to find the smallest input number. They report the code used to work in 32-bit on a couple of compilers, but it’s not reliable when used in more modern 64-bit projects.
Can you identify the problem(s), and suggest any fixes (or alternative approaches)?
The code is in Listing 2.
#include <algorithm>
#include <iostream>
// find the least value in the arguments
template <typename T, typename... Ts>
T least(T first, Ts... rest)
{
std::sort(&first,
&first + 1 + sizeof...(rest));
return first;
}
int main()
{
std::cout << "least(1): "
<< least(1) << std::endl;
std::cout << "least(1,2,3,4): "
<< least(1,2,3,4) << std::endl;
std::cout << "least(10,9,8,7,6,5,4,3,2,1): "
<< least(10,9,8,7,6,5,4,3,2,1)
<< std::endl;
}
|
| Listing 2 |
You can also get the current problem from the accu-general mail list (next entry is posted around the last issue’s deadline) or from the ACCU website (http://accu.org/index.php/journal).
This particularly helps overseas members who typically get the magazine much later than members in the UK and Europe.
Silas is a partially-sighted Computer Science post-doc in Cambridge who currently works in part-time assistant tuition. He has been an ACCU member since 1994 and can be contacted at ssb22@cam.ac.uk
Notes:
More fields may be available via dynamicdata ..