I have completed a series of updates to the http parser/generator library Luncheon but before I request the changes get pulled into the lua_libs I want to get a little more confidence in the changes. In order to do this I am asking anyone in the driver developer community to try patching this library in their drivers to see if these changes cause any issues. Directions for how to include these changes in a driver are below.
First clone the luncheon repository and checkout the misc-fixes branch and then copy the luncheon sub-directory into the driver’s src directory
git clone https://github.com/cosock/luncheon
cd ./luncheon
git checkout misc-fixes
cp ./luncheon /path/to/driver/src
As an example here is how I’ve done this in my http_button driver.
If you are using Luncheon in any of your drivers I would really appreciate some extra runtime on these changes. If you do end up trying to test this please reply here with either a success or failure.
The changes include the following
- Chunk encoding parsing on requests and responses with a
source when the Transfer-Encoding header is set to chunked
-
iter and serialize now work for both builders and source based requests and responses
-
send will call self.sock.send with up to 1024 bytes instead of just the next line
-
Headers now has an explicit append and replace method for controlling duplicate headers
- Previously multiple calls to
append would only select the last entry when serializing, now all entries will be serialized
- Request/Response helpers
set_content_length and set_content_type will now use the replace method to avoid duplicate entries for these values.
- Request/Response types now have a
replace_header method for accessing this w/o calling get_headers directly.
-
Request:content_length has been marked as deprecated in favor of Request:get_content_length which better matches the Response method.
-
Request.new no longer sets the content-length header to 0 on construction
- A large amount of the parse/generate logic has been centralized to be shared across both the
Request and Response types.
Some of the key things I am looking to get a handle on are
- Are any of the changes made non-backwards compatible?
- Missing methods or fields (anything prefixed with a
_ according to convention is not considered public)
- Does the new chunk encoding parse incoming requests/responses correctly?
- Does the changes to the
Headers type negatively impact existing uses?
Please reach out here if you have any questions and/or feedback. Thank you in advance!
3 Likes
I will definitely give it a go. I wrote my own chunked transfer logic on top of luncheon, so I’ll compare the code as well as swap it out for test.
I gave this branch a shot and the chunked encoding does work. It wasn’t a direct drop in replacement. I started by just trying to use the library without any code modifications. There were conflicts with my own chunked response handling. I had to remove my handling code and use Response.tcp_response directly. At that point it worked well with a single http request and handled the chunked transfer. However, it wasn’t compatible with my keep-alive implementation. The first request worked fine, but any other calls were “closed by peer” or had SSL errors. DM me if you want more info or code samples.
Thanks for the updates. Glad luncheon keeps evolving 
Thank you so much for the feedback! Glad things are mostly working for you.
@robert.masen I may have found an issue with the version currently on the hub, not sure if it would be applicable to your new version as well. If I call an API that returns a 404, the Response object is not reading the entire response, and leaves the remainder of the response on the socket, affecting the next REST call when it tries to read from that same socket. It doesn’t seem to do this for 200 or 207 status codes which work fine for me:
local response, err, _ = Response.source(function() return sock:receive('*l') end)
-- after this call
-- response = {_parsed_headers=false, _recvd=22, _source=function: 0x2946a70, headers={_inner={}}, http_version=1.1, status=404, status_msg="Not"}
-- err = "[unknown datatype:nil]"
Note the partial status_msg
Next call gets this prior body as an invalid preamble err:
err = 'invalid preamble: "{\"errors\":[{\"description\":\"Not Found\"}]}"'
Does the library assume a 404 will not produce a body?
Hey, thanks for the report!
I do have a fix for this issue already in the pending PR, though things may look different than they actually are. Because Luasocket is configured in your example to receive a line and not a number of bytes, it should be clearing out the Found part of that response but the status_msg will be truncated. The reason 200 and 207 aren’t impacted by this is because their status messages don’t contain a space.
Luncheon doesn’t make any assumptions about bodies based on the status code but the constructor doesn’t eagerly read the response (other than the first line), so any headers or body would be queue up in the socket and until it is cleared out it will cause the next attempt to construct a response with the same socket to read the previous responses’ contents. looking at the contents of err it seems like the request body is a json object with details about the error the server encountered. Adding a call to Response:get_body would clear that out of the socket but if that happens to be chunk encoded you’d need some special handling to ensure it doesn’t attempt receive with *a.
For 200 and 207 I get the body and have handling for chunked encoding. Sounds like I will need to run that for all error codes to clear the socket, even if I am not using the result. Does it look fine to do a full line read as I am for that response? Ive seen other ST written drivers doing this as well.
Does it look fine to do a full line read as I am for that response
Yeah, that should be fine, the default behavior for reads before we reach the end of the headers is to use the "*l" argument to receive. The place where this breaks down is on the body reading, currently Luncheon assumes "*a" for any request or response that doesn’t have a content-length header, if that header is present it will read what ever number of bytes is provided in that header.
I have also seen some replies that have an array of values for content-length. Each is the length of a chunk. Does Luncheon utilize those as well if present? So far I have had to manually read chunks from the socket myself and haven’t relied luncheon for that part.
I updated the code to get the headers and body for all status codes, and it works as you have described. The only issue now being the truncated status_msg. This is better to have the body anyway, since I can now use that for error handling if needed. Thanks.
I have also seen some replies that have an array of values for content-length. Each is the length of a chunk. Does Luncheon utilize those as well if present?
No, Luncheon takes the last value from the headers if there are multiples
The only issue now being the truncated status_msg.
If you want to resolve the full status message you can use something like
local status_msgs = require "luncheon.status"
--....
let response, err, _ = Response.status(function() sock:receive("*l"))
response.status_msg = status_msgs[response.status]
This will have the “correct” error message at the cost of potentially being different than what was sent with the first line (highly unlikely imo)
1 Like