Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Endless get request starting 0.6.7+1 version #64

Closed
sestegra opened this issue Nov 1, 2016 · 5 comments
Closed

Endless get request starting 0.6.7+1 version #64

sestegra opened this issue Nov 1, 2016 · 5 comments
Assignees
Labels
P0 A serious issue requiring immediate resolution type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@sestegra
Copy link

sestegra commented Nov 1, 2016

In my application, when a OPTIONS request is done, the server didn't answer (in the network console, the request is pending for 3 minutes).
The issue occurs on Dartium, Chrome, Safari, Firefox.

The same case is running well with previous Shelf versions.

@nex3
Copy link
Member

nex3 commented Nov 1, 2016

Can you provide a pure-Dart reproduction with a Dart HTTP client?

@nex3 nex3 added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) needs-info Additional information needed from the issue author labels Nov 1, 2016
@sestegra
Copy link
Author

sestegra commented Nov 19, 2016

See following example that works on shelf 0.6.7 and doesn't work with next version

import 'dart:async';
import 'package:shelf/shelf.dart' as shelf;
import 'package:shelf/shelf_io.dart' as io;

Future<Null> main(List<String> args) async {
  var handler = const shelf.Pipeline()
      .addMiddleware(shelf.logRequests())
      .addHandler(echoRequest);

  var server = await io.serve(handler, '127.0.0.1', 8080);
  print('Serving at http://${server.address.host}:${server.port}');
}

shelf.Response echoRequest(shelf.Request request) {
  return new shelf.Response.ok(null);
}

Use following command to simulate the request, the command doesn't complete.

$ curl -v http://127.0.0.1:8080/
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 8080 (#0)
> GET / HTTP/1.1
> Host: 127.0.0.1:8080
> User-Agent: curl/7.43.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< date: Sat, 19 Nov 2016 10:05:50 GMT
< x-frame-options: SAMEORIGIN
< content-type: text/plain; charset=utf-8
< x-xss-protection: 1; mode=block
< x-content-type-options: nosniff
< server: dart:io with Shelf
* no chunk, no close, no size. Assume close to signal end
< 

Compared to results from 0.6.7 version, the content-length: 0 part is missing in the response.

@kevmoo kevmoo added P0 A serious issue requiring immediate resolution and removed needs-info Additional information needed from the issue author labels Nov 20, 2016
@nex3 nex3 self-assigned this Nov 24, 2016
@nex3
Copy link
Member

nex3 commented Nov 24, 2016

This turns out to be pretty complex. Some of the assumptions I was making about HTTP header validity turn out to have been wrong, but fixing them will involve coming up with a consistent model for how the Content-Length and Transfer-Encoding headers interact with one another and with adapters, which is non-trivial, especially in the face of a number of dart:io issues (dart-lang/sdk#27885 and dart-lang/sdk#27886 in particular).

@Tomucha
Copy link

Tomucha commented Dec 6, 2016

I did run into it too. I'm using shelf_cors middleware, which does this:

 Response handleOptionsRequest(Request request) {
     if (request.method == 'OPTIONS') {
         return new Response.ok(null, headers: CORS_HEADERS);
         ...

Quick fix/workaround for anyone out there is to return something:

         return new Response.ok("OK", headers: CORS_HEADERS);

@nex3
Copy link
Member

nex3 commented Dec 6, 2016

I should have a fixed version of shelf released in the next couple days.

nex3 added a commit that referenced this issue Dec 6, 2016
This should be the last CL in this unfortunate series. The adapter
requirements have been updated to consistently ensure that a Message's
body is in the chunked format if and only if its headers indicate that
it should be.

Closes #64
nex3 added a commit that referenced this issue Dec 6, 2016
This should be the last CL in this unfortunate series. The adapter
requirements have been updated to consistently ensure that a Message's
body is in the chunked format if and only if its headers indicate that
it should be.

Closes #64
@nex3 nex3 closed this as completed in #66 Dec 6, 2016
nex3 added a commit that referenced this issue Dec 6, 2016
This should be the last CL in this unfortunate series. The adapter
requirements have been updated to consistently ensure that a Message's
body is in the chunked format if and only if its headers indicate that
it should be.

Closes #64
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 A serious issue requiring immediate resolution type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

4 participants