This repository has been archived by the owner. It is now read-only.

Parse Error on http.get in 0.4.11 #1569

Closed
dscape opened this Issue Aug 21, 2011 · 9 comments

Comments

Projects
None yet
3 participants
@dscape

dscape commented Aug 21, 2011

You get a parse error on this very simple request.
Works on 0.5+

// http://nodejsbug.iriscouch.com/bugreport/d481d73b6e78ed07c405293ee80be4a6/IMG_20110517_093510-1.jpeg
http = require('http');
var options = {
  host: 'nodejsbug.iriscouch.com',
  port: 80,
  path: '/bugreport/d481d73b6e78ed07c405293ee80be4a6/IMG_20110517_093510-1.jpeg'
};

http.get(options, function(res) {
  console.log(res.statusCode);
}).on('error', function(e) {
  console.log("Got error: " + e.message);
});
@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Aug 21, 2011

Member

I can confirm that it works with master but fails with v0.4.

Member

bnoordhuis commented Aug 21, 2011

I can confirm that it works with master but fails with v0.4.

@koichik

This comment has been minimized.

Show comment
Hide comment
@koichik

koichik Aug 22, 2011

The response has Content-Type header that value includes \n

Content-Type: image/jpeg;\n x-unix-mode=0600;\n name="IMG_20110517_093510-1.jpeg"

This was fixed upstream and merged into master (c3ccbea), but not yet v0.4.

koichik commented Aug 22, 2011

The response has Content-Type header that value includes \n

Content-Type: image/jpeg;\n x-unix-mode=0600;\n name="IMG_20110517_093510-1.jpeg"

This was fixed upstream and merged into master (c3ccbea), but not yet v0.4.

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Aug 22, 2011

Member

I suspected it was that.

We can't easily upgrade the HTTP parser in v0.4. It breaks four tests and the fixes may require API changes (big no no for the v0.4 branch).

Cherry-picking ry/http-parser@b89f944 works okay, all tests pass and it fixes the issue. The commit itself is side effect free and it's not an API change either so it seems like the way forward. A standalone test would be nice to have though.

Thoughts?

Member

bnoordhuis commented Aug 22, 2011

I suspected it was that.

We can't easily upgrade the HTTP parser in v0.4. It breaks four tests and the fixes may require API changes (big no no for the v0.4 branch).

Cherry-picking ry/http-parser@b89f944 works okay, all tests pass and it fixes the issue. The commit itself is side effect free and it's not an API change either so it seems like the way forward. A standalone test would be nice to have though.

Thoughts?

@koichik

This comment has been minimized.

Show comment
Hide comment
@koichik

koichik Aug 22, 2011

ry/http-parser@3258e4a?
There is only one commit between current v0.4 (ry/http-parser@eee6012) and ry/http-parser@b89f944.

koichik commented Aug 22, 2011

ry/http-parser@3258e4a?
There is only one commit between current v0.4 (ry/http-parser@eee6012) and ry/http-parser@b89f944.

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Aug 22, 2011

Member

That one's probably worth picking too.

Member

bnoordhuis commented Aug 22, 2011

That one's probably worth picking too.

@koichik

This comment has been minimized.

Show comment
Hide comment
@koichik

koichik commented Aug 22, 2011

LGTM. +1

@koichik

This comment has been minimized.

Show comment
Hide comment
@koichik

koichik Sep 2, 2011

@ry - Can you merge ry/http-parser@3258e4a and ry/http-parser@b89f944 to v0.4 branch?

koichik commented Sep 2, 2011

@ry - Can you merge ry/http-parser@3258e4a and ry/http-parser@b89f944 to v0.4 branch?

@dscape

This comment has been minimized.

Show comment
Hide comment
@dscape

dscape Sep 13, 2011

In case someone is experiencing the same problem with CouchDB the following work-around exists. CouchDB doesn't enforce newlines, this is coming from your environment when you push the attachment. CouchDB simply respects the fact that you inserted newlines in your Content-Type and preserves it.

To solve it simply remove new line chars from Content-Type.

dscape commented Sep 13, 2011

In case someone is experiencing the same problem with CouchDB the following work-around exists. CouchDB doesn't enforce newlines, this is coming from your environment when you push the attachment. CouchDB simply respects the fact that you inserted newlines in your Content-Type and preserves it.

To solve it simply remove new line chars from Content-Type.

bnoordhuis added a commit that referenced this issue Sep 14, 2011

test: add http multi-line header test
Test case for #1569. Verify that http_parser parses multi-line headers.
@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Sep 14, 2011

Member

Fixed in 8b2a335, test added in 5724b54.

Member

bnoordhuis commented Sep 14, 2011

Fixed in 8b2a335, test added in 5724b54.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.