benmerckx / monsoon

Minimal haxe web framework and embedded webserver
53 stars 8 forks source link

ByteRange.serve() won't work (always) in Safari (or Chrome) #7

Open derRaab opened 7 years ago

derRaab commented 7 years ago

Just in case somebody runs into this: The current haxelib 4.0.0 version of this project doesn't contain the current, latest sources. At least ByteRange was updated. :)

So I copy pasted the content of ByteRange to the local version and I was able to compile again.

BUT (and I think this is something that changed in Safari on macOS because it worked for a while):

These range requests seem to work on the server side but Safari doesn't handle the result properly? Maybe that is something I could fix?

That's working in Google Chrome (see the partial load progress in the "native" video controls):

bildschirmfoto 2017-02-20 um 12 29 16

And it's working in Mozilla Firefox (see the partial load progress in the "native" video controls):

bildschirmfoto 2017-02-20 um 12 29 45

It's not working in Safari (10.0.3 on latest macOS Sierra) - (see network output and "native" video controls as well):

bildschirmfoto 2017-02-20 um 12 30 02

Since I don't have much knowledge about server side development I wonder if there is something I could do?

back2dos commented 7 years ago

I guess one radical option would be not to use range serving - at least if the user agent is a Safari. If the server is local, then it really shouldn't make much of a difference anyway.

derRaab commented 7 years ago

Hm - I thought Safari demands range serving? Well, yes - one option is to disable serving over http:// completely and use the file:// protocol instead - but then people would maybe copy paste the url to a different browser (Chrome / IE) and then my local http request wouldn't work. :)

derRaab commented 7 years ago

So I thought it might be a good idea to track down the problem a little more. So I have MAMP up and running - serving the same file from exactly the same directory and MAMP does it a little different (works!):

MAMP-Request / Response (works in Safari):

bildschirmfoto 2017-02-20 um 14 28 06

Monsoom-Reqeust / Response (won't work in Safari):

bildschirmfoto 2017-02-20 um 14 28 39

Differences can be found in (german but you'll get it):

back2dos commented 7 years ago

I thought Safari demands range serving?

Well, ok, since Steve' passing Apple has shown increasing disregard for standards, but I'd be really surprised if Safari's video playback refused to work with a server that doesn't support range requests.

Range requests work like this: Client makes a request with Accept-Ranges: bytes and no Range specified and server must respond with Accept-Ranges: bytes and Range: 0-<sizeOfFirstChunk>. If there's no Accept-Ranges: bytes in the response header, the client MUST assume the server does not support ranges. Everything else is like walking into a church and ordering a beer - and expecting to get one :D

back2dos commented 7 years ago

Ok, well, looking at the difference my guess would be that the lack of a Connection header bothers Safari. Connection reuse in tink_http is still quite a bit away, so a Connection: close needs to be added to the response (I suppose).

derRaab commented 7 years ago

I gave it a try but it didn't change the behavior. Added to ByteRange.serve():

bildschirmfoto 2017-02-20 um 15 17 02
back2dos commented 7 years ago

Maybe Safari just doesn't like loading a video in ranges without connection reuse. Have you tried what it does if you disable range serving?

derRaab commented 7 years ago

Yes - I tried that again - but no - Safari requires range requests - it simply doesn't work without it.

back2dos commented 7 years ago

Phew. Ok, I'm wildly guessing here, but I suppose implementing connection reuse is the only way to solve this. You could check it by building using nodejs and .set('connection', 'keep-alive'). If that works, then that's the issue and I'm afraid I then have no easy answer for you.

benmerckx commented 7 years ago

Here's what I tested:

Neko (should have the same results as cpp)

Nodejs: both work

You don't have to change the ByteRange middleware but you can use this before initializing it:

app.use(function(req, res, next) {
    res.set('connection', 'keep-alive');
    next();
});
app.use(ByteRange.serve);
// ...
derRaab commented 7 years ago

I'll give it a try, but one question. Is it app.use( ByteRange.serve ); or app.use( ByteRange.serve() );

derRaab commented 7 years ago

Well it is app.use( ByteRange.serve() ); :) But it won't work on my machine using cpp. Headers are set like @benmerckx said. So I don't know for now. I'll prompt a proper error message regarding the usage of Safari. :D

@benmerckx : maybe you could update the haxelib version to the latest sources?

Thanks!

derRaab commented 7 years ago

I spent the whole day trying to figure this issue out, and I'm one step further, but I don't get what's wrong:

  1. Range request actually work in Safari but not with every file!
  2. The same files that won't work in Safari won't work in Chrome as well!

I have two videos within the same directory. The video that works just fine is a mp4 file with 7312432 bytes and the file that won't work is a mp4 file with just 3582329 bytes.

To my understanding range request work somewhat like that:

So I think the problem might be file size related oder first two bytes related?

Anyway - I tried to track that down modifying the ByteRange.serve() method with a lot of traces:

public static function serve()
    return function(req: Request, res: Response, next: Void -> Void) {

        trace( " - " );
        trace( " - " );
        trace( "serve() " );
        function done() {

            return Future.sync(res);
        }

        function fail() {
            res.error(416, HttpStatusMessage.fromCode(416));
            return Future.sync(res);
        }

        res.after(function (res) {

            if (res.get('content-length') == null) {

                trace( "DONE BECAUSE: res.get('content-length') == null" );
                return done();
            }

            var length = Std.parseInt(res.get('content-length'));

            trace( "RAW: res.get('content-length')", res.get('content-length') );
            trace( "Int: length", length );
            res.set('accept-ranges', 'bytes');

            var header = req.get('range');
            trace( "REQUEST req.get('range')", header );
            if (header == null) {

                trace( "DONE BACAUSE: header == null" );
                return done();
            }

            switch parseRange(req.get('range'), length) {
                case Success(ranges):

                    if (ranges.length != 1) {

                        trace( "FAIL BECAUSE: ranges.length != 1" );
                        return fail();
                    }
                    trace( "Success(ranges)", Success(ranges) );
                    trace( "ranges", ranges );
                    var range = ranges[0];
                    trace( "range", range );
                    if (range.start+range.length > length) {

                        trace( "FAIL BECAUSE: range.start+range.length > length" );
                        return fail();
                    }

                    var body: Source = res.body;
                    trace( "body:", body );
                    if (range.start > 0) {
                        trace( "range.start > 0" );
                        var limited = body.limit(range.start);
                        trace( "limited", limited );

                        limited.pipeTo(BlackHole.INST);
                        trace( "limited", limited );
                    }
                    else {

                        trace( "rang.start = 0" );
                    }

                    @:privateAccess
                    res.body = body.limit(range.length).idealize(fail);

                    res
                    .status(206)
                    .set('content-length', '${range.length}')
                    .set('content-range', 'bytes ${range.start}-${range.start+range.length-1}/${length}');

                    trace( "res", res );
                    trace( "res.body", res.body );
                    trace( "res.get('content-length' )", res.get('content-length' ) );
                    trace( "res.get('content-range' )", res.get('content-range' ) );
                    trace( "DONE()" );
                    return done();
                default:
                    trace( "FAIL()" );
                    return fail();
            }

            trace( "DONE()" );
            return done();
        });

        trace( "NEXT()" );
        next();
    }

The I tried to compare the results for both files (left one works, right version doesn't):

bildschirmfoto 2017-02-28 um 15 38 57

From line 50 it starts to differ a little. Any Ideas what that causes?

And How can I properly trace the body of the response (I wonder if the script simply sends not the correct amount of data)?

benmerckx commented 7 years ago

Does the file which fails in both chrome and safari play when served through apache (mamp)?

derRaab commented 7 years ago

@benmerckx, yes ist does.

derRaab commented 7 years ago

So, here I have some more information (from Safari).

1 A. Monsoon-Server trying to deliver the unsupported video:

bildschirmfoto 2017-03-01 um 09 15 58

1 B. MAMP-Server successfully delivering the exact same video:

bildschirmfoto 2017-03-01 um 09 15 48

2 A: Monsoon-Server successfully delivering the supported video:

bildschirmfoto 2017-03-01 um 09 22 30

2 B: MAMP-Server successfully delivering the supported video:

bildschirmfoto 2017-03-01 um 09 23 28

The only real difference I can see is the decoded size. Does this output somehow clarify this?

benmerckx commented 7 years ago

I'll try to replace the Static middleware with Kevins version, which approaches the range requests a bit differently: https://github.com/kevinresol/tink_http_middleware/blob/master/src/tink/http/middleware/Static.hx

derRaab commented 7 years ago

@benmerckx Well, I'd love to try it, but I don't fully understand the handling and since the API isn't the same I'm having a hard time figuring out how to port that over...

derRaab commented 7 years ago

...or how to use it...

benmerckx commented 7 years ago

Ok, so I've just tried it and Kevin's version seems to serve mp4 files just fine in safari and chrome. I had to push a little fix to get it to work with monsoon (so you might update to 0.5.0 first). Here's how you'd use it:

using Monsoon;

import tink.http.middleware.Static;

class Server {
  static function main() {
    var port = 5000;
    var app = new Monsoon();
    var handler = new Static('public', '/');
    app.use(handler.apply);
    app.listen(port);
    trace('Server ready and listening on http://localhost:${port}');
  }
}
derRaab commented 7 years ago

Hi again and thank you very much - I already tried it and I think this really works fine regarding range-requests! Now I need to figure out how to send 404s for non-existing files and maybe some other responses since currently the server doesn't response if the file doesn't exist. Just pending...

So I'll copy the existing code, reformat it so I can understand it better and then inject proper http responses instead of errors.

benmerckx commented 7 years ago

Well, the middleware will pass on the request if the file is not found, so you're able to do this:

// ...
app.use(handler.apply);
app.get(function(req, res) {
  res.html('<h1>Not found!</h1>');
});
// ...
derRaab commented 7 years ago

Great! I didn't know that. So I just use

app.get( function( req, res ) {
 res.status( 404 );
 res.html( '<h1>Not found</h1>' );
}

THANKS!