haxetink / tink_http Goto Github PK

It would be nice if the server would be able to listen on an IPC socket (e.g. UNIX domain socket, or a Windows named pipe).
Don't know about other platforms, but for Node.js, server.listen() supports it just as easy as listening on a TCP port, so implementation would be very simple, just need to be able to pass the path instead of port to NodeContainer.

I never used JavaServlet before. But a quick glance through some docs seem to show that it work a bit similar to php. i.e. It starts a separate process/thread for each request.

TcpContainer returns Running when setHandler returns true,
but setHandler returns false when the handler is set successfully?

As a result, TcpContainer returns Shutdown when it is actually running.

Not sure which one is wrong here.

Where are the query parameters on php?

I just realized that I have forgotten that bit.

On nodejs, multiple header values of the same name will be concatenated by ", " (doc), while on neko/php I suppose there will be multiple entries of the same header name. I am not sure if it should be standardized by tink_http or just let users handle the difference?

There are a few formats that header values can be in. Here are what I have so far encountered:

1. Plain:
   Host: localhost:8000
2. Multiple values:
   Accept-Encoding: gzip, deflate
3. Extension:
   Content-Type: multipart/form-data; boundary=--X
4. Multiple extensions:
   Content-Disposition: form-data; name="file2"; filename="a.html"
5. Multiple values with extensions:
   Accept-Language: en-US,en;q=0.5

At a glance on these variations, I propose we have the HeaderValue represented as (or get parsed as) Array<{value:String, extensions:Array<QueryStringParam>}>. Does that look appropriate?

Hi Juraj, I've watched your wwx talk. My thought afterwards was about the sharing of code between stuff that runs on tink_http. For example I've written initial gzip compression middleware for monsoon here. There's still a lot to be done there (and compression is probably better handled through nginx or the like), but aside from that I wonder if we could define some format using only tink_http requests/responses which would make it easy to create this as a seperate lib which can then for example also be used in tink_web, or ufront if/once it supports tink_http. If a common format/interface can be defined I can create a sort of wrapper/abstract in monsoon to use this kind of middleware. It would also need to allow defining an action which would run after the response is 'done', and maybe some other actions, I haven't thought that part through yet. What do you think?

I'm trying to get something very basic running:

public static function main() {
  var container = new TcpContainer(2000);
  var response: OutgoingResponse = 'it works';

  container.run({
    serve: function (x) {
      var trigger = Future.trigger();
      trace('new request');
      trigger.trigger(response);
      return trigger.asFuture();
    },
    onError: function(e) trace(e),
    done: Future.trigger()
  });
}

This compiles and runs. The first request to localhost:2000 (through a browser or wget/curl) always works, subsequent requests fail. The 'new request' trace however prints for every request, so everytime I refresh in the browser I get a line in the console, but connection drops before a response goes through. I do not get this behaviour when trying out foxhole, so I guess I'm doing something wrong here..

Any reason for the private constructor on CgiContainer?

The tests are coming along but I'm having trouble reading the body properly if it's empty. Any server based on tcpcontainer fails here, without any Success or Failure but simply hangs indefinitely. I solved this previously in monsoon in a very hacky way. But I think this should be solved properly :)

I want to use a Client to download some larger files. Seems StdClient uses Http.onData, so as far as I can tell waits for everything to be downloaded and converted to String before piping to source. Could haxe.Http.customRequest be used to create a proper body Source?

Currently errors can be handled in onError, but as far as I can tell there's no way to form a response in onError. What I'd like to do is send a 500 response with some info if something fails. Is this something that should be handled by the application inside serve? I've put a try catch in there, but that only catches synchronous errors and I'm not sure how to properly handle any asynchronous error that may occur, which I think is target specific and exactly what you're doing with onError.

On react native one can't pass binary data through the js<=>native bridge but only strings (or serializable objects). As a result, any binary data must live in the file system and referenced by a uri when sent by http. For example, to upload an image one does something like: fetch(url, {method:"post", body:{uri: local_uri_of_img}})

As a result, this doesn't match the paradigm of tink_http's client because it's body expects a source (which is binary by its nature).

So, should we support this?

If we do, I think we can change body to an enum {Binary(s:Source); Text(s:String); FormData(f:js.html.FormData;} (we need to provide a cross-platform FormData implementation here) and let the client handles them. On the other hand, for the same reason, the response body also need to be changed.

And I would like to know if there are any other exotic cases like this one on other framework/platform/etc...

As @benmerckx pointed out in #32:

The tests are becoming a huge mess :) How would you like to structure this? I think we need:

• Platform specific code in separate files
• More feedback on what test is running/failing (use a test framework)
• A way to reliably kill the server even when things go wrong (I added a try catch around the handling for mod_neko for now)

I agree the tests are messy. For me it was mostly important to have something to have some results when I went forward with the recent API changes. But if there is any chance to structure this with reasonable effort, then by all means, let's do it!

The biggest issue with using test frameworks was that we need something async and that getting buddy and tink_runloop to work with one another is everything but easy. Here's the result for tink_tcp: https://github.com/haxetink/tink_tcp/blob/master/tests/RunTests.hx

The clientIp given by NodeContainer is something like ::ffff:127.0.0.1. I think we need a bit more information about the address than just a plain string, such as the address family IPv4 or IPv6

See: https://nodejs.org/api/net.html#net_socket_remotefamily

Header names are case-insensitive according to the spec. So comparing header name in a case-sensitive way (like this) may lead to potential mismatch/bug. Especially on node js, headers are by default converted to lower case by http.Server.

We need some mechanisms to make sure things are consistent. I propose tink_http convert all header names to lower case when parsing the request, and in header.byName we lowercase the input parameter before comparing.

When updating to Haxe v3.4.7 the compilre gives this error:

tink.http.HeaderField does not have a constructor

everything works fine with Haxe v3.4.6

package;

import tink.http.Client;
import tink.http.Container;
import tink.http.Request;
import tink.http.Header;
import tink.web.Response;

using tink.CoreApi;

class Main {
    public static function main() {
        var client = new TcpClient();

        var container = new NodeContainer(8083);

        container.run({
            done: Future.trigger(),
            serve: function(req):Response return req.body.all() >>
                function(bytes:haxe.io.Bytes):Response {
                    trace(bytes.length == 0 ? 'empty bytes' : 'non-empty bytes');
                    return "done";
                },
            onError: function(e) trace(e),
        });

        var future = client.request(new OutgoingRequest(
            new OutgoingRequestHeader(POST, 'localhost', 8083, '/', null, [new HeaderField('content-type', 'application/json')]),
            haxe.Json.stringify({some:'value'})
        )) >>
            function(res:tink.http.Response.IncomingResponse) return res.body.all();

        future.handle(function(bytes) trace(bytes.sure().toString()));
    }
}

This code traces "empty bytes" but "non-empty bytes" is expected.
Tried on nodejs and neko (this particular code does not compile on neko because of NodeContainer, but wanna bring out the fact that TcpClient broke on neko too)

Incoming and outgoing request headers are currently two separate things. Investigate if that is really necessary.

How to enable compression (e.g. gzip) for the HTTP server?

To reproduce: replace this line with the following code:

https://github.com/haxetink/tink_http/blob/45b7383/tests/TestHeader.hx#L35

var trigger = Signal.trigger();
var stream = new tink.streams.Stream.SignalStream(trigger);
var req:IdealSource = stream;
trigger.trigger(Data(('GET /path HTTP/1.1\r\nHost: www.example.com\r\nUser-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 (.NET CLR 3.5.30729)\r\nAccept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8\r\nAccept-Language: en-us,en;q=0.5\r\nAccept-Encoding: gzip,deflate\r\nAccept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7\r\nKeep-Alive: 300\r\nConnection: keep-alive\r\nCookie: PHPSESSID=r2t5uvjq435r4q7ib3vtdjq120\r\nPragma: no-cache\r\nCache-Control: no-cache\r\n\r\nabc':tink.Chunk)));
// trigger.trigger(End); // uncomment to "fix"

The parser will never resolve the parsed header. I guess there is something wrong in the StreamParser or BytewiseParser

There seems to be a problem with tink_http and hxnodejs 4.0.4:

/home/jonas/haxelib/tink_http/0,1,2/src/tink/http/Container.hx:34: characters 135-152 : Null<haxe.extern.EitherType<String, Array<String>>> should be tink.http.HeaderValue
/home/jonas/haxelib/tink_http/0,1,2/src/tink/http/Container.hx:34: characters 135-152 : haxe.extern.EitherType<String, Array<String>> should be tink.http.HeaderValue
/home/jonas/haxelib/tink_http/0,1,2/src/tink/http/Container.hx:34: characters 135-152 : For function argument 'value'

Noticed this when trying out monsoon
benmerckx/monsoon#3

and Dan K pointed out that it's probably related to
HaxeFoundation/hxnodejs@fbcb80a

The secure tcpclient works in most cases. But I've found a few where the server would reply with either:

• 503 Service Unavailable
• 422 Unexpected end of input

I've also noticed the 422 error in two cases while using the tcpclient. The only workaround I've found so far is disabling the cnx.sink.close() call after body and headers are sent, which I think should signal the end of the request, and by adding extra headers (connection: close and content-length: 0).

I don't really have a clue why this happens or how to properly solve it, any ideas are welcome :) In all cases the (Secure)StdClient did work, but since it buffers the whole body (#67) it's not suitable for my needs.

This is something I keep on thinking about: if HTTP fails on a network level, shouldn't the client just return a plain error, rather than creating a fake response with an error code?

The reason I went with the current API is that:

1. It's not always easy to know whether the error came from the server or the network level. But if we get rid of StdClient, this becomes easier.
2. I found it cumbersome to have two error cases to be handled. With Promise though request(...).next(convertToErrors) is a simple way to just have Error regardless of where it came from and request(...).recover(IncomingResponse.reportError) will give you exactly the same behavior as before. But the two sources of errors nicely exist on two different levels.

Anybody have thoughts on this?

Suggest changing the api to:

function request(request:OutgoingRequest): Signal<Progress<IncomingResponse>>

and enum Progress<T> { Done(data:<T>); Progress(fraction:Float); Failed(error:Error);}

Latest haxe (4) gives me a weird compiler error:

tink/http/Header.hx:245: characters 25-33 : String should be tink.http.HeaderName
tink/http/Header.hx:245: characters 25-33 : For function argument 'name'

Tried the following which still fails:

new HeaderField((s.substr(0, v): HeaderName), s.substr(v + 1).trim());

I could only get it to compile like this:

new HeaderField(@:privateAccess new HeaderName(s.substr(0, v)), s.substr(v + 1).trim());

This is most probably be a regression in haxe, but I'm leaving this here until I can reproduce in a simpler way.

getallheaders is apache extension and maybe be undefined in other environments

There is a polyfill: https://github.com/ralouphie/getallheaders

I was trying to fix the static file middleware, where the file read stream could possibly fail (e.g. someone deleted the file while transferring / the file is in network drive and the network failed). Since the body of OutgoingResponse is an IdealStream, we have to somehow rescue the unsafe file read. But we can't simply rescue with an Empty source because that will possibly fail the client who is expecting <content-length> sized body and the transmission will be deemed as "paused" in the middle. The worst case is that the next response will be treated as the continuation of the body, if the connection is reused.

There really isn't much we can do after the header is sent out, except forcibly killing the connection. One possible solution is to make the body of OutgoingResponse a RealStream and let the container deal with the failure.

I am trying to use the Multipart parser but unable to get it working, it would be nice if you can throw some light on me.

package;

import tink.io.Source;
import tink.http.Multipart;

class Main
{
    static function main()
    {
        var body:Source = "------WebKitFormBoundaryY0Fl23M3eWoBJA4L\r\nContent-Disposition: form-data; name=\"param1\"\r\n\r\nmm\r\n------WebKitFormBoundaryY0Fl23M3eWoBJA4L\r\nContent-Disposition:
 form-data; name=\"param2\"\r\n\r\n123\r\n------WebKitFormBoundaryY0Fl23M3eWoBJA4L--\r\n";

        Multipart.parseSource(body, "----WebKitFormBoundaryY0Fl23M3eWoBJA4L")
            .forEach(function(chunk) {
                trace(chunk); 
                return true;
            }).handle(function(o) trace(o));
    }
}

TypeError: Cannot read property 'getBytes' of null

Since fields is private now, I think there should be an API for adding fields, probably cloning the header with fields.concat(added)

While trying to setup a reduced example for #40, I'm hitting the following:

Called from tink/io/Worker.hx line 10
Uncaught exception - Invalid field access : createSlave
Error: Command failed with error 1

Seems like RunLoop.current is unset. I'm getting the error when compiling for neko in all scenarios I tried. For example:

-main Main
-lib tink_http
-lib tink_tcp
-lib tink_runloop
-lib tink_concurrent
-D concurrent
-cp src
-neko main.n
-cmd neko main.n

import tink.http.containers.TcpContainer;
import tink.http.Response;
using tink.CoreApi;

class Main {
    public static function main() {
        var container: TcpContainer = new TcpContainer(8000);
        container.run(function(req) {
            trace(req);
            return Future.sync(('ok': OutgoingResponse));
        });
    }
}

I've got some very basic routing working on neko (tcp, cgi). I'd like to compile for php but I'm running into some issues.

At compile time:

• php.Web doesn't have a logMessage method, so this fails to compile. I think this could be replaced with a call to error_log.

At runtime:

• Fatal error because of the use of a yield method in tink.io.Pipe. This is a haxe compiler issue and I've submitted a PR.
• Undefined variable: hDEFAULT (errno: 8) in ../tink/io/StdSink.class.php at line #18: This is a weird error, as I can var_dump $hDEFAULT, php errors only if you try to call it. I think this is a compiler issue as well, I'll try to recreate this later and see why it goes wrong.

Once I messed around in the php output to circumvent the runtime issues, everything runs ok though :)

It fails here: https://github.com/HaxeFoundation/haxe/blob/fae543b/std/php/_std/sys/io/Process.hx#L69
with: uncaught exception: feof(): supplied resource is not a valid stream resource (errno: 2)

I checked the pointer (p) was able to respond to a feof() call in the constructor, but fails when inside the readBytes() function. Not sure if it is even related to tink_io.

This is the follow-up for #65 (comment)

I am behind an HTTP proxy that requires authentication and I need to make HTTP/HTTPS requests through the proxy in order to get to the Internet. It would be perfect if we could implement this functionality in Tink.

In particular, I would hope to be able to use it for the Lix project. I have an open issue about it here: lix-pm/lix.client#29

Just dig out an old implementation dated a few years back. Should be useful. Let's see if I (or anyone) have the time to put it into tink_http's clients.

    public function upload(fileName:String, fileContents:BytesData, ?onProgress:Pair<Int, Int>->Void):Surprise<Noise, Error> 
    {
        var boundary = Std.string(Math.round(Math.random() * 100000000));

        var data = new BytesData();
        writeString(data, '--$boundary\r\n'
            + 'Content-Disposition: form-data; name="' + PartName.UPLOAD + '"; filename="$fileName"\r\n'
            + 'Content-Type: application/octet-stream\r\n\r\n');

        writeBytes(data, fileContents);
        writeString(data, "\r\n");
        writeString(data, '--$boundary--\r\n');

        var request = new URLRequest(url);
        request.data = data;
        request.method = "post";
        request.contentType = 'multipart/form-data; boundary=$boundary';

        return Future.async(function(handler)
        {
            /*var loader = new URLLoader();
            loader.addEventListener(Event.COMPLETE, function(e)
            {
                var outcome:Outcome<Noise, Error> = Unserializer.run(e.target.data);
                trace(e.target.data);
                handler(outcome);
            });
            loader.load(request);*/

            var socket = new Socket();          
            socket.addEventListener(OutputProgressEvent.OUTPUT_PROGRESS, function(e:OutputProgressEvent)
            {
                if (onProgress != null)
                {
                    var bytesTotal = Std.int(e.bytesTotal);
                    var bytesSent =  Std.int(bytesTotal > e.bytesPending ? bytesTotal - e.bytesPending : e.bytesPending);
                    onProgress(new Pair(bytesSent, bytesTotal));
                }
            });
            socket.addEventListener(ProgressEvent.SOCKET_DATA, function(e:ProgressEvent)
            {
                var httpResponse = socket.readUTFBytes(Std.int(e.bytesLoaded));
                var httpContents = httpResponse.split("\r\n\r\n")[1];
                var outcome:Outcome<Noise, Error> = Unserializer.run(httpContents);
                handler(outcome);
                socket.close();
                socket = null;
            });

            var parts = url.split("://")[1].split("/");
            var host = parts.shift();
            var path = "/" + parts.join("/");

            socket.connect(host, 80);
            socket.writeUTFBytes('POST $path HTTP/1.0\r\n');
            socket.writeUTFBytes('Host: $host\r\n');
            socket.writeUTFBytes('Content-Length: ${data.length}\r\n');
            socket.writeUTFBytes('Content-Type: multipart/form-data; boundary=$boundary\r\n');
            socket.writeUTFBytes('Connection: Keep-Alive\r\n');
            socket.writeUTFBytes('\r\n');
            socket.writeBytes(data);
            socket.flush();

            /*var count = Std.int(data.length / 4096) + 1;
            var lastLen = data.length - 4096 * (count - 1);
            for (i in 0...count)
            {           
                socket.writeBytes(data, i * 4096, i == count - 1? lastLen : 4096);
                socket.flush();
            }*/
        });
    }

Probably a flag for the request call?

Maybe make ofError a dynamic function such that users can override it?

like so:

class Test {
    static function main() {

        I.fromString = function(s) return 2; // override the implict cast
        var i:I = 's';
        trace(i);
    }
}

abstract I(Int) from Int {
    @:from public static dynamic function fromString(v:String):I {
        return 1;
    }
}

This came up in #18.

To my understanding, this could be implemented as a function of type OutgoingResponse->OutgoingResponse. If the argument already has a Content-Length it would be returned as is, otherwise a new response would be constructed that uses chunked encoding. The tricky bit is in writing the IdealSource->IdealSource transformation for the body.

Not sure when I'll get around to do this. Volunteers are welcome. Or let me know if this becomes a pressing issue ;)

Hello,
I am trying to use this library, but fail to understand some concepts.
for example, when constructing an outgoing request, I'm supposed to provide an instance of IdealSource. I don't even know what that means. moreover most http libraries have a simple api, I'm frankly surprised at what I see here.
so please provide more explanation in the documentation or provide a simpler api to use.
Thanks!

This needs to be properly solved for devtools server and for mod_neko too, somewhere around here: https://github.com/haxetink/tink_http/blob/master/src/tink/http/containers/ModnekoContainer.hx#L47

IIRC both environments deal with multipart quite differently. If someone feels able to add the mod_neko case to the current tests, that would be a great help.

This act as a reminder for myself

On nodejs, when server.close() is called, the server does not kill existing connections but only stop accepting new ones. When all connections have been served, the server will call the callback provided in server.close(callback) as well as firing a 'close' event. I think Container should give users the ability to know such moment, because it is useful if we want to kill the process safely.

Corresponding support may be required in tink_tcp.

At here I would like to propose putting haxe.Json.stringify(e.data) instead of e.message because the message is already present as the reason.

When generating status codes from error codes, it should be % 200 + 400, to make sure it always comes out as an error code. It will thus be possible to introduce new error codes outside the HTTP range and let them be mapped to HTTP error codes nicely.

As @kevinresol pointed out in #17 there's no way to find out when the server closes. The truth is, I omitted this, because I had no good idea how to properly express it :D

Generally, the whole Container/Application API is a bit crude, to say the least. For example, nothing prevents you from running multiple applications on the same container and what not. Also, in "cgi-like" environments, you don't get to react to the server closing (which is why chose not to expose that at all).

My primary focus lies on a decent API of requests / responses and good implementations of the protocol itself, which is why I have neglected Container so far. But we should definitely deal with it. Maybe @benmerckx also has some input on that subject?

I think I would prefer to get rid of Application altogether, as it doesn't compose well (it's cumbersome to plug together two apps currently - further below I will illustrate how I feel it should be). So something like this might be good:

interface Container {
  function run(handler:Handler):Future<ContainerResult>;
}

typedef Handler = {
  IncomingRequest->Future<OutgoingResponse>;
}

enum ContainerResult {
  Failed(e:Error);//failed to bind port, multiple apps on the same container, or whatever
  Running(running:RunningState);//for node, tink_tcp, ...
  Done;//for cgi-like stuff
}

interface RunningState {
  function shutdown():Future<Noise>;
  var failures:Signal<ResponseFailure>;
}

typedef ResponseFailure = {
  request: IncomingRequest,
  response: IncomingResponse,
  error: Error,
}

It's a bit fatter than the status quo, but I think it does model the different scenarios somewhat closer.

Building on the above, this is what I mean by easier composition:

typedef Condition = IncomingRequest->Future<Bool>;

function combine(condition:Condition, consequence:Handler, alternative:Handler):Handler
  return function (req:IncomingRequest) 
    return condition(req).flatMap(
      function (applies) return 
        if (applies) consequence(req)
        else alternative(req)
    )

function route(?method:Method->Bool, prefix:String):Condition
  return function (req:IncomingRequest) return Future.sync(
     (method == null || method(req.header.method)) 
       && 
     req.header.url.startsWith(prefix)
  );

function route(prefix:String)
  return rule(function (m) return m == GET, prefix);

function host(expected:String):Rule
  return function (req:IncomingRequest) return Future.sync(
    switch req.header.get('host') {
      case [found] if (found == expected): true;
      default: false;
    }
  );

//here's some routing:
combine(
  host('api.myapp.com'),
  apiHandler,
  combine(
    get('/static/'),
    serveStatic,
    webHandler
  )
);
//or why not middle wares?
typedef Preprocessor = IncomingRequest->Future<IncomingRequest>;
typedef Postprocessor = OutgoingResponse->Future<OutgoingResponse>;

function preprocess(p:Preprocessor, h:Handler):Handler 
  return function (req) return p(req).flatMap(h);

function postprocess(p:Postprocessor, h:Handler):Handler 
  return function (req) return h(req).flatMap(p);

//and use them here

This is just an example of how nicely handlers could potentially be strung together, to create complex apps. Not sure the code even compiles and I don't mean to discuss it's specifics, only to illustrate what could be won by having composable handlers, rather than the bulky Application. Also, it seems a good idea to separate life cycle management for request handling. But I probably digress.

Anyway, my main point is, the current API needs revision anyway and I would like to replace it by something better, that does not just deal with the particular issue @kevinresol raised, but is something we can commit to.

So please pour all your thoughts here!

I think we should really put the clients into individual files. The Client.hx is getting kinda big.

It puts e.message as response reason, they may fail on nodejs under some circumstances (not sure if it is utf8 or some special symbols)

Will come back with further investigation ...