Parse Status Messages to makte them available in Node.js #147

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

VanCoding commented Jan 17, 2013

I've requested a feature on node.js where I can access the status message of responses, so I decided to add this feature to the http_parser.

joyent/node#4614

@@ -204,6 +204,8 @@ struct http_parser {
unsigned short http_major;
unsigned short http_minor;
unsigned short status_code; /* responses only */
+ char status_msg[4096]; /* responses only */
@bnoordhuis

bnoordhuis Jan 17, 2013

Member

Sorry, I can't take this. http_parser tries very hard remain small and lightweight. sizeof(http_parser) == 32 bytes on x86_64 and now you add a 4 kB buffer?

Member

bnoordhuis commented Jan 17, 2013

Sorry, this is not an acceptable change. Closing.

@bnoordhuis bnoordhuis closed this Jan 17, 2013

Contributor

VanCoding commented Jan 18, 2013

No problem. So what would be the right way to solve this?
It's also not acceptable to not implement this, since it's part of the HTTP spec.

If it were in C++, I could use a string, which is dynamically alocated, but it C there is not much else I could do.

Contributor

clifffrey commented Jan 18, 2013

The right way would be to add another callback, just like is done for the
header lines or the body.

On Thu, Jan 17, 2013 at 4:23 PM, Patrik Stutz notifications@github.comwrote:

No problem. So what would be the right way to solve this?
It's also not acceptable to not implement this, since it's part of the
HTTP spec.

If it were in C++, I could use a string, which is dynamically alocated,
but it C there is not much else I could do.


Reply to this email directly or view it on GitHubhttps://github.com/joyent/http-parser/pull/147#issuecomment-12399711.

Contributor

VanCoding commented Jan 18, 2013

I've changed it now to the "right" way described by @clifffrey.
Since you closed the pull request, the commits don't seem to update anymore...

Maybe reopen it?

My changes currently compile, but I don't know if they really work since I have no clue how to test it :D

Member

bnoordhuis commented Jan 18, 2013

Can you submit it as a new PR? As to tests, take a look at how the other callbacks are tested in test.c.

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